Skip to content

Various updates for --progress-fd #961

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

Merged
merged 5 commits into from
Jan 9, 2025
Merged

Conversation

antheas
Copy link
Contributor

@antheas antheas commented Dec 11, 2024

deploy: Avoid cloning progress

Have the printer take ownership and return it. This is generally
prep for making the writer more stateful which will make it easier
to work with.

Signed-off-by: Colin Walters [email protected]


cli: Rename --json-fd to --progress-fd

The format is not as important as what the option does. There's
multiple things that can be JSON, but this is specifically
about dynamic progress information.

Signed-off-by: Colin Walters [email protected]


cli: Make --progress-fd hide=true for now

We're still debating some of the interface/semantics here
so let's mark this as hidden.

Signed-off-by: Colin Walters [email protected]


docs: Describe --progress-fd, add rendered JSON schema

Signed-off-by: Antheas Kapenekakis [email protected]
Signed-off-by: Colin Walters [email protected]


@antheas antheas changed the title docs(progress): Adds docs for JSON progress format docs(progress): Add docs for JSON progress format Dec 11, 2024
@cgwalters
Copy link
Collaborator

I did some tweaks here, though we still need to generate a schema for the progress and probably clarify even more strongly the guidelines for how we evolve it.

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Jan 7, 2025
@antheas
Copy link
Contributor Author

antheas commented Jan 7, 2025

Would also be good to decide if you prefer org.containers.bootc/progress/v1 or org.containers.bootc.progress/v1 and match them. The code uses the latter, the docs the former.

@cgwalters cgwalters self-assigned this Jan 7, 2025
@cgwalters cgwalters mentioned this pull request Jan 7, 2025
3 tasks
@cgwalters cgwalters force-pushed the progress-docs branch 2 times, most recently from 280f659 to 17a656a Compare January 8, 2025 18:02
@cgwalters
Copy link
Collaborator

Would also be good to decide if you prefer org.containers.bootc/progress/v1 or org.containers.bootc.progress/v1 and match them. The code uses the latter, the docs the former.

I think we should use a semantic version here instead (without any namespace prefixing because it's obvious the data here is coming from bootc), and a semantic version would allow us to better track protocol evolutions (e.g. a client can know to expect new fields). That said of course in practice JSON often used precisely because it's "loose" and new fields can be added at any time.

Really also instead of having a version number in each event, I think it'd make sense to emit that version once as a special event type.

cgwalters and others added 4 commits January 8, 2025 13:24
Have the printer take ownership and return it. This is generally
prep for making the writer more stateful which will make it easier
to work with.

Signed-off-by: Colin Walters <[email protected]>
The format is not as important as what the option *does*. There's
multiple things that can be JSON, but this is specifically
about dynamic progress information.

Signed-off-by: Colin Walters <[email protected]>
We're still debating some of the interface/semantics here
so let's mark this as hidden.

Signed-off-by: Colin Walters <[email protected]>
Signed-off-by: Antheas Kapenekakis <[email protected]>
Signed-off-by: Colin Walters <[email protected]>
@cgwalters
Copy link
Collaborator

Really also instead of having a version number in each event, I think it'd make sense to emit that version once as a special event type.

Pushed a commit here that does that.

And use a semantic version for the API version as this allows
a clearer evolution.

Signed-off-by: Colin Walters <[email protected]>
@cgwalters cgwalters enabled auto-merge January 8, 2025 21:48
@cgwalters cgwalters changed the title docs(progress): Add docs for JSON progress format Various updates for --progress-fd Jan 8, 2025
@cgwalters cgwalters requested a review from jeckersb January 9, 2025 14:29
Copy link
Collaborator

@jeckersb jeckersb left a comment

Choose a reason for hiding this comment

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

LGTM, just two typo nits

While the `bootc status` tooling allows a client to discover the state
of the system, during interactive changes such as `bootc upgrade`
or `bootc switch` it is possible to monitor the status of downloads
or other operations at a fine-grained level with `-progress-fd`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing a - in --progress-fd

// For messages that can be dropped, if we already sent an update within this cycle, discard this one.
// TODO: Also consider querying the pipe buffer and also dropping if we can't do this write.
// TODO: Also consider querying the pipe buffer and also dropping if wqe can't do this write.
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo snuck in here

@cgwalters cgwalters merged commit 255b743 into bootc-dev:main Jan 9, 2025
25 checks passed
@jeckersb
Copy link
Collaborator

jeckersb commented Jan 9, 2025

missed that automerge was on, will file typo fix followup :)

jeckersb added a commit to jeckersb/bootc that referenced this pull request Jan 9, 2025
Signed-off-by: John Eckersberg <[email protected]>
cgwalters added a commit that referenced this pull request Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants