-
Couldn't load subscription status.
- Fork 6
chore: support v2 jobs in tests #108
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
base: main
Are you sure you want to change the base?
Conversation
ca1c0ac to
d83365c
Compare
|
Appbundle tests are failing for Julia 1.6, possibly because the appbundles prepared in that version of julia is not compatible in the job environment running a very different version of julia. We should probably skip the appbundle tests for 1.6 then? Nightly is running into some strange errors, looks like a messed up depot on runner? |
src/jobs/logging-legacy.jl
Outdated
|
|
||
| function JobLogMessage(::_LegacyLogging, json::Dict, offset::Integer) | ||
| message = _get_json(json, "message", String) | ||
| message = _get_json_or(json, "message", String, "") |
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.
While https://github.com/JuliaComputing/JuliaHub/pull/20558 should ensure that message attribute exists for all log records streamed from aws cloudwatch backend (and kafka backend already does so), would like to retain this bit here to make sure the client still handles systems that do not have the patch.
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.
Fair enough. But let's add a comment.
| message = _get_json_or(json, "message", String, "") | |
| # The .message property _should_ always be present in the log messages, | |
| # but there are a few versions out there where it's sometimes omitted due | |
| # to a backend bug. So we default to an empty string in those cases. | |
| message = _get_json_or(json, "message", String, "") |
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.
That said, it's also now a behavior change, and should be PR-ed separately and have a CHANGELOG note..
|
The failure on Julia nightly is because of this: JuliaLang/julia#59956 |
Updating tests to support v2 jobs and recent changes to logging and job managelemt.