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

DEV-973: check correct journal file for continue #63

Merged
merged 2 commits into from
Dec 17, 2024

Conversation

aelkiss
Copy link
Member

@aelkiss aelkiss commented Dec 13, 2024

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.

@aelkiss aelkiss requested a review from moseshll December 13, 2024 21:39
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)
Copy link
Member Author

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

@coveralls
Copy link

coveralls commented Dec 13, 2024

Coverage Status

coverage: 92.483%. remained the same
when pulling d8839d2 on DEV-973-check-correct-journal
into f73dca4 on main.

@aelkiss aelkiss requested a review from mwarin December 16, 2024 14:34
@aelkiss aelkiss mentioned this pull request Dec 16, 2024
Copy link
Contributor

@mwarin mwarin left a 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)
Copy link
Contributor

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)

@aelkiss aelkiss merged commit 80237bd into main Dec 17, 2024
1 check passed
@aelkiss aelkiss deleted the DEV-973-check-correct-journal branch December 17, 2024 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants