Dropdb should be forced to close any active connections and not fail #213
Conversation
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
| def wipe_development_database | ||
| Kernel.system( | ||
| "dropdb --if-exists #{development_db} && createdb #{development_db}", | ||
| "dropdb --if-exists #{development_db} --force && createdb #{development_db}", |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [85/80]
…ort specific backup IDs. Added logging for restoration processes and updated tests to verify new behavior.
| log_restore_info | ||
| reset_remote_database | ||
| # Filter out --backup-id from additional_args as it's handled separately | ||
| filtered_args = additional_args.gsub(/--backup-id\s+\S+/, '').strip |
There was a problem hiding this comment.
Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| def restore_from_local_temp_backup | ||
| puts "Restoring backup to #{development_db}..." | ||
| # Filter out --backup-id from additional_args as it's not needed for pg_restore | ||
| filtered_args = additional_args.gsub(/--backup-id\s+\S+/, '').strip |
There was a problem hiding this comment.
Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
|
|
||
| def restore_from_local_temp_backup | ||
| puts "Restoring backup to #{development_db}..." | ||
| # Filter out --backup-id from additional_args as it's not needed for pg_restore |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [85/80]
| else | ||
| puts "Downloading latest backup from #{from}..." | ||
| Kernel.system( | ||
| "curl -o tmp/latest.backup \"$(heroku pg:backups:url --remote #{from})\"", |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [84/80]
| if backup_id | ||
| puts "Downloading backup #{backup_id} from #{from}..." | ||
| Kernel.system( | ||
| "curl -o tmp/#{backup_id}.backup \"$(heroku pg:backups:url #{backup_id} --remote #{from})\"", |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [103/80]
| # Filter out special flags that are handled separately | ||
| filtered_args = arguments.drop(1) - ["--force", "--parallelize"] | ||
| # Filter out --backup-id as it's handled separately by the Backup class | ||
| filtered_args = filtered_args.reject { |arg| arg.start_with?("--backup-id") } |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [83/80]
| ).restore | ||
| } | ||
| backup_args[:backup_id] = backup_id? if backup_id? | ||
|
|
There was a problem hiding this comment.
Layout/TrailingWhitespace: Trailing whitespace detected.
| "--dbname #{default_db_name} --jobs=#{cores} " | ||
| end | ||
|
|
||
| def specific_backup_id_restore_from_local_temp_backup_command(cores: number_of_processes) |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [91/80]
| end | ||
|
|
||
| def specific_backup_id_download_command | ||
| 'curl -o tmp/b001.backup "$(heroku pg:backups:url b001 --remote production)"' |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [81/80]
| with(create_heroku_ext_schema_command) | ||
| expect(Kernel). | ||
| to have_received(:system). | ||
| with(specific_backup_id_restore_from_local_temp_backup_command(cores: 1)) |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [81/80]
| else | ||
| puts "Downloading latest backup from #{from}..." | ||
| Kernel.system( | ||
| "curl -o tmp/latest.backup \"$(heroku pg:backups:url --remote #{from})\"", |
There was a problem hiding this comment.
Could you use the same curl statement in both places since a nil backup_id would be an empty string here? This would avoid having two pieces of interpolated string arguments to maintain going forward where we could have one.
|
|
||
| alias :parallelize? :parallelize | ||
|
|
||
|
|
There was a problem hiding this comment.
Trim these down, please.
| if backup_id_index && backup_id_index + 1 < arguments.length | ||
| arguments[backup_id_index + 1] | ||
| end | ||
| end |
There was a problem hiding this comment.
The predicate method should return a boolean result. Given that you're using the ID returned here I think the ? should be dropped from the method name.
2 participants
This pull request includes a small change to the
restore_to_developmentmethod inlib/parity/backup.rb. The change ensures that thedropdbcommand is executed with the--forceflag to enforce the removal of the development database even if there are active connections.