-
Notifications
You must be signed in to change notification settings - Fork 2
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
DEV-973: check correct journal file for continue #63
Conversation
it "indexes it and writes the journal" do | ||
latest_examples = CICTL::Examples.of_type(:upd, :delete).select { |e| e[:date] == "20230103" } | ||
|
||
CICTL::Examples.of_type(:full, :upd) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no journals for delete examples
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me. Zero gripes, just a suggested improvement outside the changed code.
@@ -15,6 +15,15 @@ def metrics? | |||
metrics.match?(/^job_records_processed\S*job="#{job_name}"\S* \S+/m) | |||
end | |||
|
|||
# records may be updated multiple times, so we need to dedupe those | |||
def unique_id_count(examples) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the new unique_id_count
could also be used on line 145:
- expect(solr_count).to eq examples.map { |ex| ex[:ids] }.flatten.uniq.count
+ expect(solr_count).to eq unique_id_count(examples)
cictl index continue
was checking only that the full journal existed (and checking that multiple times)... This should correct the observed behavior where it is not indexing additional daily files after doing the full run.