-
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
Create, Update backfill after MMF run #1299
Conversation
m := &pb.Match{ | ||
MatchId: "1", | ||
Tickets: []*pb.Ticket{t1}, | ||
Backfill: b, |
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.
Please test the mmf changing a value on the backfill.
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 will check changes to search fields and extensions fields will be applied.
client, err := om.Query().QueryTickets(ctx, &pb.QueryTicketsRequest{Pool: &pb.Pool{ | ||
StringEqualsFilters: []*pb.StringEqualsFilter{{StringArg: "field", Value: "value"}}, | ||
}}) | ||
|
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.
We're also going to want to include making sure the backfill updates in query. (having queried before so we know it has the old cached version.) This isn't going to work until several pieces are in place though... just make sure it's on the work items that get done. (either your tracker, or just an issue on github)
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.
You are talking about querying tickets or backfills? Using such code I just want to ensure that ticket associated with backfill will disappear from query results.
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.
Oh actually this just made me realize, when a backfill is rejected because of an update from the game server - we need to release the tickets because they were already marked as pending by the synchronizer. (or move the marking as pending to the backend after verifying the backfill would work too.)
I was talking about the backfill having a new field value set by the mmf.
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.
The work re: unpending tickets because a backfill changed, can be done in a followup PR.
From the CI failure: |
What this PR does / Why we need it:
Each time when MMF proposes a match with backfill, the backfill should be saved or updated in the store.