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

Improved Docker and cloud support #318

Merged
merged 1 commit into from
Apr 16, 2021
Merged

Conversation

anders-kiaer
Copy link
Collaborator

@anders-kiaer anders-kiaer commented Sep 30, 2020

Closes #150.

  • Remove dependency on webviz base image.
  • Use same Python slim base image as user had when creating the portable app.
  • Install same webviz framework version as user has locally.
  • Install same plugin projects, with same version, as user has locally.
  • Create Radix config automatically.

@anders-kiaer anders-kiaer changed the title Add Azure blob storage support Improved Docker and cloud support Oct 2, 2020
@anders-kiaer anders-kiaer force-pushed the azure branch 2 times, most recently from e267919 to 4435bad Compare October 7, 2020 07:57
@anders-kiaer anders-kiaer self-assigned this Oct 19, 2020
@anders-kiaer anders-kiaer force-pushed the azure branch 3 times, most recently from c67f92c to 94dd0c7 Compare November 9, 2020 20:54
@anders-kiaer anders-kiaer force-pushed the azure branch 2 times, most recently from e3f0c14 to 51c1a5f Compare November 12, 2020 19:30
@anders-kiaer anders-kiaer marked this pull request as ready for review November 12, 2020 19:30
HansKallekleiv
HansKallekleiv previously approved these changes Nov 13, 2020
Copy link
Collaborator

@HansKallekleiv HansKallekleiv left a 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 :)

@anders-kiaer anders-kiaer force-pushed the azure branch 3 times, most recently from f526edd to 4ede579 Compare November 19, 2020 13:28
@anders-kiaer anders-kiaer dismissed HansKallekleiv’s stale review November 23, 2020 08:15

We decided to increase scope of PR (due to recent improvements in Radix)

@anders-kiaer anders-kiaer marked this pull request as draft December 9, 2020 10:50
@anders-kiaer anders-kiaer force-pushed the azure branch 3 times, most recently from 07f0f28 to 9a46f59 Compare December 13, 2020 22:33
Comment on lines 53 to 58
resources:
requests:
memory: "400Mi"
cpu: "100m"
limits:
memory: "4G"
cpu: "1000m"
Copy link
Collaborator

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?

Copy link
Collaborator Author

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.

@anders-kiaer anders-kiaer assigned asnyv and unassigned anders-kiaer Jan 24, 2021
@anders-kiaer anders-kiaer assigned anders-kiaer and unassigned asnyv Mar 19, 2021
@anders-kiaer anders-kiaer force-pushed the azure branch 3 times, most recently from 5e9f930 to e751af8 Compare March 22, 2021 19:06
@anders-kiaer anders-kiaer force-pushed the azure branch 5 times, most recently from 06b75e6 to 5fc794d Compare April 13, 2021 18:16
@anders-kiaer anders-kiaer marked this pull request as ready for review April 13, 2021 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Install Python packages in Dockerfile
3 participants