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

[#11497] Fix server error when retrieving log entries from cloud logging #11714

Merged
merged 2 commits into from
Apr 10, 2022

Conversation

halfwhole
Copy link
Contributor

@halfwhole halfwhole commented Apr 9, 2022

Fixes #11497

The issue should occur due to the call to iterateAll() happening after logging.close(), so this fix shifts the call to iterateAll to before closing the logging resource. (props to @t-cheepeng for identifying the issue)

Have tested on staging and the functionality still works as before, but since I can't reproduce the issue on my staging will need some external help to check that the fix works as intended.

@halfwhole halfwhole added the s.ToReview The PR is waiting for review(s) label Apr 9, 2022
@wkurniawan07
Copy link
Member

wkurniawan07 commented Apr 9, 2022

@halfwhole possible to treat this as a hot-patch candidate and rebase this PR against release instead?

Edit: will be making a fresh release from master instead.

@wkurniawan07 wkurniawan07 added this to the V8.13.1 milestone Apr 9, 2022
Copy link
Member

@wkurniawan07 wkurniawan07 left a comment

Choose a reason for hiding this comment

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

A little bit of duplications, but the focus right now is to ensure that the issue is fixed first.

@wkurniawan07 wkurniawan07 added c.Bug Bug/defect report s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.ToReview The PR is waiting for review(s) labels Apr 10, 2022
@wkurniawan07 wkurniawan07 merged commit b5adbbc into TEAMMATES:master Apr 10, 2022
ziqing26 pushed a commit to ziqing26/teammates that referenced this pull request Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c.Bug Bug/defect report s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Instructor viewing activity logs: server error
2 participants