Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recreate cron indexes to be conditional #1163

Merged

Conversation

defkode
Copy link
Contributor

@defkode defkode commented Nov 27, 2023

fixes #1161

@bensheldon
Copy link
Owner

bensheldon commented Nov 27, 2023

@defkode sorry, I realized I have so much experience doing this :D

  • I think this can be a single migration.
  • Indexes are a little weird to do idempotently because each call needs to be wrapped with an if index_exists? check because the migration could fail out somewhere in the middle because of the disable_ddl! removing the transaction wrapper.
  • You'll also need to update the templates/install migration
  • Then you'll need to run the template generator on the demo app to generate the live database migration files. Then run those which will update the demo app's schema.rb

...so ultimately this should be updates to 4 files:

  • lib/generators/good_job/templates/update/migrations/06_recreate_good_job_cron_indexes.rb
  • lib/generators/good_job/templates/install/migrations/06_recreate_good_job_cron_indexes.rb
  • demo/db/migrate/xxxxx_recreate_good_job_cron_indexes.rb
  • demo/db/schema.rb

@defkode defkode force-pushed the 1161_partial_indices_for_cron_key branch from d23ff87 to baec364 Compare November 28, 2023 07:59
@defkode defkode changed the title recreate_index_good_jobs_on_cron_key Recreate good_jobs cron indexes Nov 28, 2023
@defkode
Copy link
Contributor Author

defkode commented Nov 28, 2023

@bensheldon Thanks for hints. PR is updated.

@defkode defkode marked this pull request as ready for review November 28, 2023 08:04
@bensheldon bensheldon changed the title Recreate good_jobs cron indexes Recreate cron indexes to be conditional Dec 1, 2023
…ve concurrent from initial install migration
@bensheldon
Copy link
Owner

@defkode thank you! I just made a few tweaks. I'm going to let CI run through 💚 and then it should be done ⭐

@bensheldon
Copy link
Owner

I'm pretty certain this is failing because of a change introduced today on Rails main: rails/rails#50205 (comment)

@defkode
Copy link
Contributor Author

defkode commented Dec 1, 2023

@bensheldon Thanks!

@bensheldon bensheldon added enhancement New feature or request bug Something isn't working and removed enhancement New feature or request labels Dec 1, 2023
@bensheldon bensheldon merged commit 7133bd0 into bensheldon:main Dec 1, 2023
20 checks passed
bensheldon added a commit that referenced this pull request Dec 2, 2023
bensheldon added a commit that referenced this pull request Dec 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging this pull request may close these issues.

Use partial indices for cron_key?
2 participants