-
Notifications
You must be signed in to change notification settings - Fork 115
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
chore: Update Pulumi Java SDK to v0.8.0 #2337
Conversation
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
1 similar comment
Does the PR have any schema changes?Looking good! No breaking changes found. |
@@ -123,7 +123,7 @@ javadoc { | |||
if (JavaVersion.current().isJava9Compatible()) { | |||
options.addBooleanOption('html5', true) | |||
} | |||
options.jFlags("-Xmx2g", "-Xms512m") | |||
options.jFlags("-Xmx8g", "-Xms512m") |
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 rely on having that much on CI runners?
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.
Assume this will just act as unbounded and will fail if the host won't give it more memory - this is only the max and not the inital memory to be reserved?
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 is max, yes
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.
Is the extra memory required? I'm not opposed to the change, but curious what the motivation is.
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.
Think this is generated by the java language - was increased because some providers (probably AWS or Azure) crash with only 2gb
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.
Yes, these changes come from upstream (in the make build
, particularly, building the Java sdk step) when we updated to the latest Java gen version. Xmx indicates the maximum allowed, so it won't fail on start, only if the program requires more and the host can't allocate it.
This looks like it takes the form
|
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.
Added a question that's purely interrogatory - wouldn't consider it a blocker as the rest of the PR looks good to me.
Fixes #2333
Steps carried out in this PR:
make build