-
Notifications
You must be signed in to change notification settings - Fork 73
fix: Branch blockers API extraction and processing #2183
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
Conversation
Signed-off-by: ch097711 <[email protected]>
Signed-off-by: ch097711 <[email protected]>
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.
This looks good. I'm a bit unsure if we should land this because we're having some issues with storage size -- the branch blockers can be quite heavy and we need to watch out for taking up a lot of space.
Signed-off-by: ch097711 <[email protected]>
e6cd5d5
to
f3d95f9
Compare
I noticed a comment in the # Dump things we dont want to accummulate.
#save_branch_blockers(branch_pairs, project_name) Line 462 in b194efb
However, this line is not commented out in the Line 751 in b194efb
If there's still a concern about storage and a need to prevent saving this blocker information, maybe we could comment out all calls to Thanks! |
Yeah, there's still a concern. Could you do this though? Then I'll run this locally and do some analysis on the memory consumption -- if it looks good we can enable it |
…entation for the `/api/branch-blockers` endpoint Signed-off-by: ch097711 <[email protected]>
I've commented out the Thanks! |
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.
actually, a thing we could do to avoid not providing the data is to lazily fetch the branch blockers in the webapp, could you do this? So that when an API request comes in that uses branch blockers, we'll dynamically fetch it when it's needed
Previously, branch_blocker information was parsed from
summary.json
. However, a previous commit moved the branch_blocker information tobranch-blockers.json
, but the parsing logic was not updated accordingly. As shown in the figure, this API has no return value.Therefore, this PR fixes the parsing of branch_blocker data to now be extracted from
branch-blockers.json
.