Skip to content
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

Generalize UI events for cataloging tasks #2369

Merged
merged 5 commits into from
Nov 30, 2023
Merged

Generalize UI events for cataloging tasks #2369

merged 5 commits into from
Nov 30, 2023

Conversation

wagoodman
Copy link
Contributor

@wagoodman wagoodman commented Nov 28, 2023

This PR replaces the set of events relating to cataloging down to a single event type, setting up #1383 for generalized cataloging tasks. There is no functional change to the appearance of the UI with this code change:

Screenshot 2023-11-28 at 4 24 53 PM

In the future this code reorg sets up for a set of cataloger tasks that may be related to one another in a tree-like form, expressed in the UI (see an example in anchore/bubbly#29).

Detailed changes:

  • removes the PackageCatalogerStarted, FileMetadataCatalogerStarted, and FileDigestsCatalogerStarted event types, handlers, and related tests. These are being replaced with the existing CatalogerTaskStarted event.
  • Extends the CatalogerTaskStarted event handling with a new stateful handler. The handler creates and maintains a single catalogerTaskState instance. This instance is responsible for injecting new models into the bubbletea event loop.
  • Adds a simpleHandler adapter for the existing handlers that do not raise up tea.Cmds with new models (a change brought in by Add tree-of-models bubble component bubbly#29)
  • Expands monitor.GenericTask to be able to specify a task ID, a parent ID, and limited formatting options
  • Updates the bubbletea test helpers (such as flatten) based on the latest bubbletea lib changes
  • Made a small enhancement to the UI to comma-format long numbers show as counts of objects (such as packages and digests)

This is a breaking change since some underlying event types have been removed from the API.

(This is work broken off of #1383 )

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
@wagoodman wagoodman added the breaking-change Change is not backwards compatible label Nov 28, 2023
@github-actions github-actions bot removed the breaking-change Change is not backwards compatible label Nov 28, 2023
@wagoodman wagoodman marked this pull request as ready for review November 28, 2023 21:41
@wagoodman wagoodman requested a review from a team November 28, 2023 21:58
@wagoodman wagoodman added the breaking-change Change is not backwards compatible label Nov 28, 2023
@wagoodman wagoodman enabled auto-merge (squash) November 28, 2023 22:46
Copy link
Contributor

@spiffcs spiffcs left a comment

Choose a reason for hiding this comment

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

Small nits, but if changes not needed or can be left alone feel free to auto merge =)

cmd/syft/cli/ui/handle_cataloger_task.go Outdated Show resolved Hide resolved
Default: "Downloading go mod",
},
HideOnSuccess: true,
ParentID: "TODO", // TODO: setting this to non-empty will cause the progress bar to be nested, but this needs to be more specific
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Is this TODO for solving in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I need to remove the value, but this will be taken care of fully in #1383 . The impact is that this will be another sibling task and not nested under the "cataloging packages" task.

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Copy link
Contributor

@kzantow kzantow left a comment

Choose a reason for hiding this comment

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

👍

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
@wagoodman wagoodman enabled auto-merge (squash) November 30, 2023 16:19
@wagoodman wagoodman merged commit 4adfbeb into main Nov 30, 2023
10 checks passed
@wagoodman wagoodman deleted the flatten-ui-events branch November 30, 2023 16:25
spiffcs added a commit to coheigea/syft that referenced this pull request Dec 5, 2023
* main: (40 commits)
  chore(deps): bump anchore/sbom-action from 0.15.0 to 0.15.1 (anchore#2392)
  Retrieve remote licenses using pom.properties when there is no pom.xml (anchore#2315)
  fix(java): improve identification for org.apache.tapestry artifacts (anchore#2384)
  fix(java): improve identification for io.ratpack artifacts (anchore#2379)
  fix(java): improve identification for org.apache.cassandra artifacts (anchore#2386)
  fix(java): improve identification for org.neo4j.procedure artifacts (anchore#2388)
  fix: bump fangs for ptr summarize fix (anchore#2387)
  fix(java): improve identification for org.elasticsearch artifacts (anchore#2383)
  fix(java): improve identification for org.apache.geode artifacts (anchore#2382)
  fix(java): improve identification for org.apache.tomcat.embed artifacts (anchore#2381)
  fix(java): improve identification for io.projectreactor.netty artifacts (anchore#2378)
  fix(java): improve identification for org.eclipse.platform artifacts (anchore#2349)
  Generalize UI events for cataloging tasks (anchore#2369)
  chore(deps): update tools to latest versions (anchore#2376)
  chore(deps): bump github.com/google/go-containerregistry (anchore#2377)
  chore: fix tests failing due to Mac Rosetta cache (anchore#2374)
  fix: improve dotnet portable executable identification (anchore#2133)
  fix file metadata cataloger to use resolved locations (anchore#2370)
  fix: logging level for parsing potential PE files (anchore#2367)
  only remove breaking-change label when there are schema changes (anchore#2371)
  ...

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
* generalize ui events for cataloging tasks

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* moderate review comments

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* incorporate review comments

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* rename cataloger task progress object

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* migrate cataloger task fn to bus helper

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

---------

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Change is not backwards compatible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants