-
Notifications
You must be signed in to change notification settings - Fork 326
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
Implement backfill querying #1310
Conversation
Attaching the gist of the diff, part of the code where moved into a separate file |
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.
Approving as it is the same as previous #1300.
@@ -162,15 +162,15 @@ func (om *om) MMFConfigHTTP() *pb.FunctionConfig { | |||
// that parsing bugs can't hide logic bugs. | |||
const registrationInterval = time.Millisecond * 200 | |||
const proposalCollectionInterval = time.Millisecond * 200 | |||
const pendingReleaseTimeout = time.Millisecond * 200 |
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.
Do the backfill tests need a larger timeout? Given that this behavior may impact both backfill / non backfill scenarios - whats the impact of this on the regular e2e tests etc.?
(I see this being used in TestTicketReleaseByTimeout / TestReleaseTickets etc.)
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.
@yeukovichd addressed this in Slack before:
The reason why TestProposedBackfillUpdate is failing time to time because sometimes it takes a little bit longer than pendingReleaseTimeout (200ms in tests) to create\update backfill after tickets added to pending release in synchroniser.
What this PR does / Why we need it:
PR allows querying and caching backfills.
A take over of #1300 and #1280 also has @Laremere comments.