-
Notifications
You must be signed in to change notification settings - Fork 276
Add contest export script #3039
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
f55ed07 to
d13e643
Compare
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.
Some small comments but I like it
misc-tools/export-contest.in
Outdated
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.
Initially we pass '' in default_name, so this becomes .{i}? (same with .{key} below)
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.
Hmm, I think you're right. The idea is that this builds up a default file name while recursively descending into the object's structure. So at the base level you always find the object itself as a dict, and that means that at line 94 default_name is set to the property key, but as you say with a prefix .. So for example for multiple JPEG files in the array under contest.logo, their default names would become .logo.0.jpg, .logo.1.jpg. So that first . must indeed be stripped.
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.
Fixed by using default argument default_name = None and adding . based on that.
misc-tools/export-contest.in
Outdated
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.
Why use store_path here? Can't we just use name below?
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.
Yeah, I think so. I guess this is a left-over of previous refactoring. It's a bit annoying that there's too many similar concepts floating around: name of the endpoint, endpoint path in the URL, path to store the downloaded endpoint data to on disk, and filename on disk.
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.
Replaced store_path by name and documented the parameters better.
71e854b to
e93273e
Compare
2485a42 to
1b40824
Compare
...via CLI Fetching the event feed from the API via `internalApiRequest` wasn't working since it returns a StreamedResponse. Fix this by only using an output buffer in `internalApiRequest` and only calling `ob_flush` from `getEventFeedAction` if the top-level output buffer allows it. Also return contents from `internalApiRequest` without JSON decoding it, since the event feed endpoint is NDJSON, *not* JSON. Explicitly decode the JSON at the other places calling internalApiRequest`.
This will be used by the contest export script for downloading the event feed (NDJSON) and other files. Also improve error reporting when an internal API call fails.
Otherwise the original error is hidden making this test failing very hard to debug. This should also be improved in the other tests that follow redirects.
1b40824 to
f0b1b51
Compare
Closes #2726