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

Use rails enum for error_event and lock_type #1420

Merged
merged 2 commits into from
Jul 15, 2024

Conversation

Earlopain
Copy link
Collaborator

@Earlopain Earlopain commented Jul 15, 2024

A bit more cleanup now that v4 is released.

Rails 7.2 will deprecate the legacy enum notation but the new one was only introduced in Rails 7.0 so a bit of version introspection is needed here.

I also had to notice that proper validations are only available in rails 7.1. I think that's fine, the test matrix will take care of this but its not as nice as I expected it would be.

There was a duplicate table_name in process.rb, I've removed that in the process of this.

@Earlopain Earlopain force-pushed the rails-enum branch 2 times, most recently from 5afd568 to 66e2fbb Compare July 15, 2024 10:29
@bensheldon
Copy link
Owner

I also had to notice that proper validations are only available in rails 7.1. I think that's fine, the test matrix will take care of this but its not as nice as I expected it would be.

In older versions it should raise an ArgumentError right? I'm sorta mixed on whether removing the constants in favor of strings+validations is equivalent in protection against fat fingering it, but it's probably ok 👍🏻

@bensheldon bensheldon added the refactor Code changes that do not introduce new features label Jul 15, 2024
@bensheldon bensheldon merged commit e7cd7d7 into bensheldon:main Jul 15, 2024
23 of 25 checks passed
@Earlopain
Copy link
Collaborator Author

Yeah, that's right. You would get an ArgumentError on 6.1 at assign time (i.e. execution.error_type = :fiscarded). With the validate option on newer rails you get a proper RecordInvalid error.

@Earlopain Earlopain deleted the rails-enum branch July 15, 2024 13:34
Earlopain added a commit to Earlopain/good_job that referenced this pull request Aug 6, 2024
2 enum values conflict with already existing scopes: "retried" and "discarded" (bensheldon#1420)
"running" and "finished" got moved from the discrete execution (bensheldon#1427)
bensheldon added a commit that referenced this pull request Aug 8, 2024
* Fix a few method redefinition warnings

2 enum values conflict with already existing scopes: "retried" and "discarded" (#1420)
"running" and "finished" got moved from the discrete execution (#1427)

* Use the simpler version of finished; use queued/running without checking advisory locks

---------

Co-authored-by: Ben Sheldon [he/him] <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code changes that do not introduce new features
Projects
Development

Successfully merging this pull request may close these issues.

2 participants