kpt alpha rpkg update --discover: option to find downstream packages that can be updated from an upstream package
#3443
Conversation
…ted from an upstream package
natasha41575
requested review from
droot,
justinsb,
mengqiy,
mortent and
yuwenma
as code owners
internal/cmdrpkgupdate/discover.go
Outdated
| return err | ||
| } | ||
| } else { | ||
| if _, err := fmt.Fprintln(w, "UPSTREAM PACKAGE\tUPSTREAM REVISION\tDOWNSTREAM PACKAGE\tDOWNSTREAM REVISION"); err != nil { |
There was a problem hiding this comment.
The last column "DOWNSTREAM REVISION" isn't actually the package revision number of the downstream revision, it's the upstream revision number that the downstream package revision is based on - but I was struggling to think of a clear, concise way to specify that in the column name. It'll probably be good to document it somewhere what these column names actually mean.
There was a problem hiding this comment.
Yeah, I agree the terminology is a bit difficult here.
When the user specifies --discovery=upstream, we currently list packages (using just "PACKAGE" as the column heading). I think maybe we should just use the heading "PACKAGE" in the --discovery=downstream case as well, since having columns named both "UPSTREAM PACKAGE" and "DOWNSTREAM PACKAGE" becomes a bit confusing.
For the "DOWNSTREAM REVISION" column, could we maybe name it something like "FROM VERSION" or "INCLUDES VERSION" to make it clearer that this is the version of the upstream that is included in the downstream package.
There was a problem hiding this comment.
When the user specifies --discovery=upstream, we currently list packages (using just "PACKAGE" as the column heading). I think maybe we should just use the heading "PACKAGE" in the --discovery=downstream case as well, since having columns named both "UPSTREAM PACKAGE" and "DOWNSTREAM PACKAGE" becomes a bit confusing.
👍 I will make it consistent with --upstream
For the "DOWNSTREAM REVISION" column, could we maybe name it something like "FROM VERSION" or "INCLUDES VERSION" to make it clearer that this is the version of the upstream that is included in the downstream package.
I like "FROM VERSION". Taking that idea a bit further, maybe we could combine the revision columns:
$ kpt alpha rpkg update --discover=downstream
PACKAGE REVISION DOWNSTREAM PACKAGE DOWNSTREAM UPDATE
kpt-samples-b2d61a19750d212c61a1c64b0a1cf1ce0efddc2 test-deployments-2f36052b298673c231e8debcdd1a4553200b45c7 v1->v2
My most recent commit does this. WDYT?
natasha41575
changed the title
kpt alpha rpkg update --discover: option to find downstream packages that can be updated from an upstream packagekpt alpha rpkg update --discover: option to find downstream packages that can be updated from an upstream package
natasha41575
changed the title
kpt alpha rpkg update --discover: option to find downstream packages that can be updated from an upstream packagekpt alpha rpkg update --discover: option to find downstream packages that can be updated from an upstream package
| var updates [][]string | ||
| var upstreamUpdates [][]string | ||
| downstreamUpdatesMap := make(map[string][]porchapi.PackageRevision) // map from the upstream package revision to a list of its downstream package revisions | ||
|
|
There was a problem hiding this comment.
Would it be easier to just create separate functions for the upstream and downstream cases here? The switch statement inside the loop will always have the same value for every iteration. I think it would also remove the need for two separate switch statements (finding updates and printing the results).
There was a problem hiding this comment.
done! Thanks for the suggestion, the code is a lot cleaner this way.
internal/cmdrpkgupdate/discover.go
Outdated
| return err | ||
| } | ||
| } else { | ||
| if _, err := fmt.Fprintln(w, "UPSTREAM PACKAGE\tUPSTREAM REVISION\tDOWNSTREAM PACKAGE\tDOWNSTREAM REVISION"); err != nil { |
There was a problem hiding this comment.
Yeah, I agree the terminology is a bit difficult here.
When the user specifies --discovery=upstream, we currently list packages (using just "PACKAGE" as the column heading). I think maybe we should just use the heading "PACKAGE" in the --discovery=downstream case as well, since having columns named both "UPSTREAM PACKAGE" and "DOWNSTREAM PACKAGE" becomes a bit confusing.
For the "DOWNSTREAM REVISION" column, could we maybe name it something like "FROM VERSION" or "INCLUDES VERSION" to make it clearer that this is the version of the upstream that is included in the downstream package.
…s that can be updated from an upstream package (kptdev#3443)
Related to #3202
Trying to tackle the second bullet point - finding out which downstream packages need an update based on the provided upstream package.
This PR does the following:
What is currently supported by
kpt alpha rpkg update --discoverwill change tokpt alpha rpkg update --discover=upstream:This PR adds another option
--discover=downstreamto tackle the reverse question. If a package revision has multiple downstream packages that need to be updated, those entries in the output table will be grouped together (like the middle two in the following output):You can optionally provide package revisions as arguments if you only want to look at specific ones:
If there is nothing to output, we just print that all downstream packages are up to date: