-
Notifications
You must be signed in to change notification settings - Fork 39
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
Improved Docker and cloud support #318
Conversation
e267919
to
4435bad
Compare
c67f92c
to
94dd0c7
Compare
e3f0c14
to
51c1a5f
Compare
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.
Looks good!
I am not entirely sure that the radix configuration
should be included in webviz-config
as this is Equinor specific? Could instead the webviz-config-equinor
be extended ? Something we can come back to.
Besides that only a minor comment to the documentation :)
f526edd
to
4ede579
Compare
We decided to increase scope of PR (due to recent improvements in Radix)
07f0f28
to
9a46f59
Compare
resources: | ||
requests: | ||
memory: "400Mi" | ||
cpu: "100m" | ||
limits: | ||
memory: "4G" | ||
cpu: "1000m" |
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 it ideal to default these settings?
I guess we can override them after initial setup, but we've seen significantly higher memory usage on localhost than what has been defaulted here?
Also: I would think that such a spread between requested and maximum memory usage could lead to out of memory errors if you need the limit value, and you're a bit unlucky?
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.
I think it is outside scope in this PR to intelligently find good values based on what the user has added to the app. But we should review the default values before merge 👍 https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ and https://www.radix.equinor.com/docs/reference-radix-config/#resources give some information.
5e9f930
to
e751af8
Compare
06b75e6
to
5fc794d
Compare
Closes #150.