fix(jobs): accept managed_load as --job-type filter (#160)#195
Merged
Conversation
The jobs list crash on managed_load rows was already resolved by the SDK 0.6.0 bump (c803d22), whose JobType enum carries the ManagedLoad variant the server emits for `databases load`. This closes the remaining gap: the CLI's --job-type filter still rejected managed_load because both the clap value_parser and parse_job_type were stale (last touched when the SDK only had 4 user-facing types). managed_load is intentionally customer-visible: runtimedb's is_maintenance() (#740) hides only the 5 internal maintenance jobs from listings, and managed_load is deliberately not among them. Displayed TYPE stays the raw token, consistent with the other job types. Adds regression tests proving a managed_load row deserializes and that the filter token maps to JobType::ManagedLoad.
Contributor
There was a problem hiding this comment.
Change is correct and well-scoped: managed_load is added to both the clap value_parser and parse_job_type, with regression tests covering deserialization and parsing.
super nit (not blocking): two in-repo docs still list the stale --job-type set and now omit managed_load:
README.md:299—--job-typeaccepts:data_refresh_table,data_refresh_connection,create_index.skills/hotdata/SKILL.md:294— same list.
The PR's Follow-ups section only calls out the external docs site (www.hotdata.dev); these two are in-repo and directly affected, so worth updating here while you're in the area.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #160.
TL;DR
The reported crash is already fixed; this PR closes the remaining filter gap.
What #160 actually is, after investigation
The issue was filed against CLI 0.5.0 / SDK 0.3.1. Verifying against current
main(not the issue body):0.6.0(commitc803d22, shipped in 0.10.0), whoseJobTypeenum includesManagedLoad. Amanaged_loadrow injobs listnow deserializes cleanly instead of failing the whole response. Proven by a new test (list_response_with_managed_load_deserializes) that passed before this PR's code change.runtimedb/openapi.yamlalready defines allJobTypevariants and the SDK (generated inhotdata-dev/sdk-rust) matches it.--job-type managed_loadwas still rejected — both the clapvalue_parser(command.rs) andparse_job_type(jobs.rs) were stale, last touched when the SDK had only 4 user-facing types.Why
managed_loadis correct to surface to customersruntimedb's
is_maintenance()(#740) deliberately hides only the 5 internal maintenance jobs (ducklake_vacuum,ducklake_orphan_cleanup,result_deletion,stale_result_cleanup,result_retention) from listings — andmanaged_loadis intentionally not among them. It's the job created byhotdata databases load. The displayed TYPE stays the rawmanaged_loadtoken, consistent with the other four job types (no friendly-label layer exists today; introducing one is a separate, all-types decision).Changes
src/command.rs: addmanaged_loadto the--job-typevalue_parser.src/jobs.rs: mapmanaged_load→JobType::ManagedLoadinparse_job_type; add two regression tests.Verification
cargo test --bin hotdata: 237 passed.cargo clippy: clean on touched files.hotdata jobs list --job-type managed_loadis accepted; invalid values listmanaged_loadamong possible values.Follow-ups (out of scope, not done here)
www.hotdata.dev/content/docs/cli-reference.mdlist the same stale--job-typeset and should addmanaged_load.#[serde(other)]catch-all inhotdata-dev/sdk-rust's generatedJobTypeso future unknown server job types degrade gracefully instead of crashingjobs list.