Skip to content

Remove the unused --output-dir flag; clarify project and custom-image guidance - #315

Merged
divyashreepathihalli merged 2 commits into
keras-team:mainfrom
JyotinderSingh:cli-cleanup-and-clearer-errors
Aug 19, 2026
Merged

Remove the unused --output-dir flag; clarify project and custom-image guidance#315
divyashreepathihalli merged 2 commits into
keras-team:mainfrom
JyotinderSingh:cli-cleanup-and-clearer-errors

Conversation

@JyotinderSingh

Copy link
Copy Markdown
Collaborator

Description

jobs_options declared --output-dir (envvar KINETIC_OUTPUT_DIR), but its only consumer, kinetic jobs list, never read the parameter. The flag was inert: KINETIC_OUTPUT_DIR is resolved at submit time in backend/execution.py, not by any jobs subcommand. This change drops the option and mark the CLI column n/a in the configuration precedence table.

The "project is required" errors named only --project and KINETIC_PROJECT, skipping the primary path: cli/main.py injects active profile fields into Click's default_map, so kinetic init is what most users want. Both the CLI (_ensure_project) and library (get_required_project) messages now lead with it.

remote_runner.py imports absl at module load, so a custom image without absl-py fails at pod startup before the payload is unpickled. Name it in the --dockerfile help, the build_and_push_prebuilt_image docstring, and the containers guide. The auto-generated image is unaffected: _CORE_DEPS pins keras-kinetic, which depends on absl-py.

Contributor Agreement

  • I am a human, and not a bot.
  • I will be responsible for responding to review comments in a timely manner.
  • I will work with the maintainers to push this PR forward until submission.
  • I will test the changes on my cloud setup and provide proof of successful validation.

… guidance

jobs_options declared --output-dir (envvar KINETIC_OUTPUT_DIR), but its
only consumer, `kinetic jobs list`, never read the parameter. The flag
was inert: KINETIC_OUTPUT_DIR is resolved at submit time in
backend/execution.py, not by any jobs subcommand. Drop the option and
mark the CLI column n/a in the configuration precedence table.

The "project is required" errors named only --project and
KINETIC_PROJECT, skipping the primary path: cli/main.py injects active
profile fields into Click's default_map, so `kinetic init` is what most
users want. Both the CLI (_ensure_project) and library
(get_required_project) messages now lead with it.

remote_runner.py imports absl at module load, so a custom image without
absl-py fails at pod startup before the payload is unpickled. Name it in
the --dockerfile help, the build_and_push_prebuilt_image docstring, and
the containers guide. The auto-generated image is unaffected: _CORE_DEPS
pins keras-kinetic, which depends on absl-py.
Apply the docs style rules to the prose from the previous commit: active
voice, one idea per sentence, no passive constructions, and no gerund
chains.

- checkpointing.md: replace "by passing/by setting" and "Both are read"
  with imperative steps and an active statement of when Kinetic reads
  the value.
- configuration.md: name the subject instead of the pronoun, and state
  the missing CLI flag as a fact about Kinetic.
- containers.md and the --dockerfile help: split "must install X and
  COPY Y" into two sentences, because install and copy are separate
  actions. Say why the three packages are mandatory.

Assert the package list against the authored help string. Click wraps at
hyphens, so a rendered "google-cloud-storage" can span two lines.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request removes the unused --output-dir CLI option from the kinetic jobs commands and updates the documentation and tests accordingly. It also adds absl-py to the list of required packages for custom Dockerfiles and updates the error messages when no GCP project is configured to guide users to run kinetic init first. There are no review comments, so no feedback is provided.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 2.70270% with 36 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@a6adb41). Learn more about missing BASE report.

Files with missing lines Patch % Lines
kinetic/constants_test.py 0.00% 14 Missing ⚠️
kinetic/cli/commands/jobs_test.py 0.00% 12 Missing ⚠️
kinetic/cli/commands/build_image_test.py 0.00% 10 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #315   +/-   ##
=======================================
  Coverage        ?   36.74%           
=======================================
  Files           ?       83           
  Lines           ?    15250           
  Branches        ?        0           
=======================================
  Hits            ?     5603           
  Misses          ?     9647           
  Partials        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@divyashreepathihalli
divyashreepathihalli merged commit 4644906 into keras-team:main Aug 19, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants