Convert C2M to worker Queue #1062
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
atz
force-pushed
the
c2m_quuuuu
branch
2 times, most recently
from
baf9b83
to
4b5c471
Compare
This was referenced Aug 8, 2018
SaravShah
approved these changes
Aug 8, 2018
app/jobs/catalog_to_moab_job.rb
Outdated
| raise ArgumentError, 'CompleteMoab param required' unless job.arguments.first.is_a?(CompleteMoab) | ||
| end | ||
|
|
||
| # @param [CompleteMoab] complete_moab object to checksum |
There was a problem hiding this comment.
assuming this is copy pasta
- Now uses factory that is fixture-aware. - Use single `AuditResults` instance double for all cases. - Stop intercepting `UnrelatedModel.new(...)` and just set the double object directly into the subject where needed. Then set expecations on the model's getter (instead of on the double independently). - More better `before` blocks
jmartin-sul
approved these changes
Aug 9, 2018
| @@ -56,8 +53,6 @@ class CompleteMoab < ApplicationRecord | |||
| ' AND (last_checksum_validation + (fixity_ttl * INTERVAL \'1 SECOND\')) < CURRENT_TIMESTAMP'\ | |||
| ' OR last_checksum_validation IS NULL' | |||
| ) | |||
| # possibly counter-intuitive: the .order sorts so that null values come first (because IS NOT NULL evaluates | |||
| # to 0 for nulls, which sorts before 1 for non-nulls, which are then sorted by last_checksum_validation) | |||
There was a problem hiding this comment.
small nitpick (leftovers from the prior PR that got rid of sort order on the scope): it'd be nice to move these comments to the class methods that are now provided as ordering wrappers.
|
merged, happy to do my nitpick as a follow-on if others find it agreeable. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note: we already abandoned order when we moved order from scopes to class methods. The service classes still called the scopes and not the class methods, so no ordering was happening.
That's more obvious now, since moving to a queue means asynchronicity inherently. Order enqueued does not guarantee order of completion. Also presumably parallelization provides better enough performance and reliability that we aren't worried about some objects never making it through.
Massive cleanup to twisty code. No entanglement of class methods and instance methods. A factory for fixtures!
Follow on work would include:
cleanup README, maybe wiki