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

FISH-10319 Add create-jvm-option command #7139

Merged
merged 3 commits into from
Jan 14, 2025

Conversation

Viii3
Copy link
Member

@Viii3 Viii3 commented Dec 19, 2024

Description

  • Resolves FISH-10319
    • Adds create-jvm-option asadmin command.
    • Adds relevant manpage.

Important Info

Blockers

Testing

New tests

Testing Performed

Used the command to create a new JVM option, verified the existence of the new option via the list-jvm-options command.

Testing Environment

Maven version: 3.9.6
Java version: 11.0.23, vendor: Eclipse Adoptium
Default locale: en_GB, platform encoding: Cp1252
OS name: "windows 11", version: "10.0", arch: "amd64", family: "windows"

Documentation

Notes for Reviewers

throw new IllegalArgumentException(msg);
}
if (bag.contains(option)) {
// setting an option that already exists is considered an error
Copy link
Member

Choose a reason for hiding this comment

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

Probably outside the scope of this issue, but this doesn't seem to work unless the string matches exactly.
For example:

asadmin create-jvm-option -XX:MaxPermSize=512m
Successfully added JVM option: -XX:MaxPermSize=512m

asadmin create-jvm-option -XX:MaxPermSize=513m
Successfully added JVM option: -XX:MaxPermSize=513m

-XX:MaxPermSize=513m
remote failure: JVM option -XX:MaxPermSize=513m already exists in the configuration.

Co-authored-by: Andrew Pielage <pandrex247@hotmail.com>
@Pandrex247 Pandrex247 added the PR: DO NOT MERGE Don't merge PR until further notice label Jan 14, 2025
@Pandrex247
Copy link
Member

Code freeze missed, do not merge until RC cut

@Pandrex247
Copy link
Member

New RCs will be cut

@Pandrex247 Pandrex247 removed the PR: DO NOT MERGE Don't merge PR until further notice label Jan 14, 2025
@Pandrex247 Pandrex247 merged commit d87a554 into payara:main Jan 14, 2025
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants