Skip to content

chore: remove schema 4 feature flag and temp reprocess citations batch job #6369

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

Merged
merged 6 commits into from
Jan 3, 2024

Conversation

nayib-jose-gloria
Copy link
Contributor

@nayib-jose-gloria nayib-jose-gloria commented Dec 18, 2023

Reason for Change

Changes

  • remove schema 4 feature flag usages
  • removed schema 4 feature flag from tests, FeatureFlagService
  • update FeatureFlagService tests
  • remove temp reprocess citations batch job

Testing steps

  • automated tests

Checklist 🛎️

Notes for Reviewer

Copy link
Contributor

Deployment Summary

@nayib-jose-gloria nayib-jose-gloria marked this pull request as ready for review December 18, 2023 22:06
Copy link

codecov bot commented Dec 18, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6e44c58) 91.99% compared to head (9dc45b4) 92.01%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6369      +/-   ##
==========================================
+ Coverage   91.99%   92.01%   +0.01%     
==========================================
  Files         180      180              
  Lines       14803    14789      -14     
==========================================
- Hits        13618    13608      -10     
+ Misses       1185     1181       -4     
Flag Coverage Δ
unittests 92.01% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nayib-jose-gloria nayib-jose-gloria changed the title chore: remove schema 4 feature flag chore: remove schema 4 feature flag and temp reprocess citations batch job Dec 19, 2023
Copy link
Contributor

github-actions bot commented Jan 3, 2024

This PR has not seen any activity in the past 2 weeks; if nobody comments or reviews it in the next 3 days, this PR will be closed.

@github-actions github-actions bot added the stale label Jan 3, 2024
}
})
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nayib-jose-gloria @Bento007 are we sure we want to remove this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought when we were quarterbacking this at least one of you was a strong proponent of building this solution in to keep around. @Bento007 don't you have a slice of code you were working on to do some aspect of the reprocessing? Can't remember where you ended up on that...

Copy link
Contributor Author

@nayib-jose-gloria nayib-jose-gloria Jan 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Bento007 made a point that keeping this around while we're not using it or working on productionalizing it just creates extra AWS job defs every time we create an rdev. Should we want to pick it up again and make it robust, it'd require enough of a rewrite that this code wouldn't be super useful anyway (most of the useful stuff is copied from the dataset metadata update batch job, so it is captured there should we want to expand that)

Copy link
Contributor

@danieljhegeman danieljhegeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving though consider whether or not the reprocessing batch job should actually be removed, as commented on

@nayib-jose-gloria nayib-jose-gloria merged commit 25fb62d into main Jan 3, 2024
@nayib-jose-gloria nayib-jose-gloria deleted the nayib/schema-4-cleanup branch January 3, 2024 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants