feat!: Support adaptive chunking, general CLI improvement #138
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mwiebe
force-pushed
the
adaptive-chunking-and-more
branch
3 times, most recently
from
1f0f6a3 to
4a2fbaa
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
crowecawcaw
previously approved these changes
Mar 4, 2025
* Add an --extensions option that can select which Open Job Description extensions to enable. By default all extensions that are implemented are enabled. * Validate that task parameters provided from the CLI are within the parameter space specified by the job template. * Modify LocalSession from a "deferred" style to "immediate." Previously it queued up all the session actions, then ran them. To be adaptive, it needs to run some session actions and use their timings to change the chunk size, and that's not compatible with pre-determining them all. * Add the ability to run multiple steps in the same session when they include step environments. It exits any step environments for a step before proceeding to the next one. * Change the default timestamps to be relative to session start, to make local debugging easier. Add an option to control the formatting between relative, local, and utc. * Modify __main__.py to be more idiomatic by removing the 'if __name__ == "__main__"' and moving the main() function out. * Create an openjd.cli.main function to call with the CLI arguments. * Modify a number of tests to use a more end-to-end CLI main function approach. * Bump openjd-model and openjd-sessions dependency versions to include the required changes. BREAKING CHANGE: The logging output has changed to use relative timestamps by default, and print more messages about the job and steps that are running. Signed-off-by: Mark Wiebe <399551+mwiebe@users.noreply.github.com>
mwiebe
force-pushed
the
adaptive-chunking-and-more
branch
from
4a2fbaa to
772d311
Compare
|
crowecawcaw
approved these changes
Mar 4, 2025
miabatta
approved these changes
Mar 7, 2025
Contributor
miabatta
left a comment
miabatta
left a comment
There was a problem hiding this comment.
Looks great! Thanks so much.
|
|
||
| def test_openjd_run_on_chunked_job_adaptive_chunking(capsys): | ||
| # Test that running chunked_job.yaml with adaptive chunking and the TargetRuntime cranked really high | ||
| # resutls in two chunks, the first being one task, and the second being the remainder. |
Contributor
There was a problem hiding this comment.
nit: spelling :)
Approving anyway since this is a big change, I'm sure there's more stuff to fix/improve at the same time if we ever do a docs pass of this repo
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What was the problem/requirement? (What/Why)
The TASK_CHUNKING RFC has been implemented and merged into openjd-model-for-python. #134 enabled minimal support, but does not support the adaptive chunk runtime option from the RFC.
In order to enable adaptive chunking, a larger refactor was necessary to move from pre-queuing all actions to instead determine them dynamically. I added more usability improvements to the scope of this refactor.
What was the solution? (How)
__main__.pyto be more idiomatic by removing the'if __name__ == "__main__"'and moving the main() function out.What is the impact of this change?
The CLI command supports chunking more fully. It also has better error messages, more informative log messages, has the ability to run a whole job by skipping the
--stepoption, and can run multiple steps in the same session without the previous limit.How was this change tested?
Added tests for the new functionality. Changed many tests to be more end-to-end instead of testing implementation details based on argparse.
Was this change documented?
Docstrings, CLI help output for new options.
Is this a breaking change?
BREAKING CHANGE: The logging output has changed to use relative
timestamps by default, and print more messages about the job and
steps that are running.
Does this change impact security?
No, it doesn't change the threat modeling.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.