Dropdb should be forced to close any active connections and not fail#213
Dropdb should be forced to close any active connections and not fail#213sethhorsley wants to merge 4 commits intothoughtbot:mainfrom
Conversation
…ort specific backup IDs. Added logging for restoration processes and updated tests to verify new behavior.
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
|
|
||
| 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]
lib/parity/backup.rb
Outdated
|
|
||
| alias :parallelize? :parallelize | ||
|
|
||
|
|
There was a problem hiding this comment.
Trim these down, please.
| # 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]
lib/parity/backup.rb
Outdated
| 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.
lib/parity/backup.rb
Outdated
|
|
||
| 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.
- Replace 'alias' with 'alias_method' for better Ruby style - Standardize string continuation formatting across all Kernel.system calls - Improve method chaining readability in development_db method - Add Rails environment loading before reading database.yml to support credentials - Use consistent double quotes for string literals
| fetch(DATABASE_KEY_NAME) | ||
| YAML.safe_load(database_yaml_file, aliases: true) | ||
| .fetch(DEVELOPMENT_ENVIRONMENT_KEY_NAME) | ||
| .fetch(DATABASE_KEY_NAME) |
There was a problem hiding this comment.
Layout/DotPosition: Place the . on the previous line, together with the method call receiver.
| fetch(DEVELOPMENT_ENVIRONMENT_KEY_NAME). | ||
| fetch(DATABASE_KEY_NAME) | ||
| YAML.safe_load(database_yaml_file, aliases: true) | ||
| .fetch(DEVELOPMENT_ENVIRONMENT_KEY_NAME) |
There was a problem hiding this comment.
Layout/DotPosition: Place the . on the previous line, together with the method call receiver.
| "heroku pg:backups:restore #{backup_from} --remote #{to} "\ | ||
| "#{additional_args}", | ||
| "heroku pg:backups:restore #{backup_from} --remote #{to} " \ | ||
| "#{filtered_args}" |
There was a problem hiding this comment.
Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.
| "#{additional_args}", | ||
| "pg_restore tmp/#{backup_filename} --verbose --no-acl --no-owner " \ | ||
| "--dbname #{development_db} --jobs=#{processor_cores} " \ | ||
| "#{filtered_args}" |
There was a problem hiding this comment.
Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.
| 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.
Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.
Metrics/LineLength: Line is too long. [83/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.
Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.
Metrics/LineLength: Line is too long. [102/80]
| "heroku pg:reset --remote #{to} #{additional_args} "\ | ||
| "--confirm #{heroku_app_name}", | ||
| "heroku pg:reset --remote #{to} #{additional_args} " \ | ||
| "--confirm #{heroku_app_name}" |
There was a problem hiding this comment.
Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.
| 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.
Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.
Metrics/LineLength: Line is too long. [84/80]
| "heroku pg:push #{development_db} DATABASE_URL --remote #{to} "\ | ||
| "#{additional_args}", | ||
| "heroku pg:push #{development_db} DATABASE_URL --remote #{to} " \ | ||
| "#{additional_args}" |
There was a problem hiding this comment.
Style/TrailingCommaInArguments: Put a comma after the last parameter of a multiline method call.
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.