-
-
Notifications
You must be signed in to change notification settings - Fork 28
Implement update to specific version in the ProsperoInstallationManager (wildlfy-core req) #979
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
base: main
Are you sure you want to change the base?
Conversation
|
CI jobs are expected to fail. |
03919a5 to
99bb26c
Compare
a158f5c to
6276e1c
Compare
|
@TomasHofman we need to approve this as well? |
bebf757 to
622457e
Compare
|
@pedro-hos yes please, review this one as well, I was just waiting for merging of Bartek's PR to rebase this one. |
|
@yersan I'm keeping this one as draft until we have wildfly-installation-manager-api release ready, but it's ready for review as well. |
|
CI is going to keep failing because there is dependency on SNAPSHOT version of installation-manager-api. |
|
@yersan actually you don't have to review this one (unless you want to), it's OK if you just review the wildfly-core and installation-manager-api. My mistake... :) |
622457e to
e7d47fd
Compare
pedro-hos
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, only that small change.
| } | ||
| String newVersion = manifestVersion.getVersion(); | ||
| String oldVersion = channelVersion.getPhysicalVersion(); | ||
| if (VersionMatcher.COMPARATOR.compare(newVersion, channelVersion.getPhysicalVersion()) < 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use the oldVersion instead of the channelVersion.getPhysicalVersion() into this if condition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will fix, thanks!
e7d47fd to
9427433
Compare
yersan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While getting familiar with the changes, I've noticed that the help provides unexpected description for the version argument
$ prospero.sh update perform --help
Applies the latest available updates directly on the server instance.
Usage: prospero update perform [--dir=<path>] [OPTION]...
(to update a server installation)
or: prospero update perform --self [--dir=<path>] [OPTION]...
(to update prospero)
Options:
...
--version=<versions>[,<versions>...]
Prints the version of prospero and exits.
...
|
Thanks @yersan ! While possible to fix the wrong version help description, there is indeed the top level command --version parameter for printing the prospero version. I believe functionally it's not an issue, but I feel like it would be better to have distinct parameter names for these two functionalities. I would rename |
|
|
…manifest-versions
Example: $ ./prospero.sh history [e7fd6d5b] 2026-01-29T14:37:45Z - update * channels:wildfly-core:1.2 * org.wildfly.channels:wildfly-ee:39.0.0.Final (WildFly EE 39.0.0.Final) * org.wildfly.prospero:prospero-wildfly-galleon-pack:1.4.0.Beta3-SNAPSHOT [90097877] 2026-01-28T16:10:04Z - install * channels:wildfly-core:1.1 * org.wildfly.channels:wildfly-ee:39.0.0.Final (WildFly EE 39.0.0.Final) * org.wildfly.prospero:prospero-wildfly-galleon-pack:1.4.0.Beta3-SNAPSHOT
|
BTW I added the renaming commit, plus some improvement of history command output |
|
If it is not yet, wildFly proposal should be updated accordingly |
| } else { | ||
| final String versions = manifestVersions.stream().map(Version::getDisplayVersion).collect(Collectors.joining("+")); | ||
| msg = "[" + versions + "]"; | ||
| return String.format("[%s] %s - %s", hash, timestamp.toString(), type.toString().toLowerCase(Locale.ROOT)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change left an unused import
import java.util.stream.Collectors;
This is preparation for the implementation in wildfly-core (JBoss CLI).
Only the last commit is relevant, the other commits are from the as yet unmerged feature PR.