Add datasets-server-worker to the Kube cluster #236
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
also: fix the dev configuration (typo on the name)
this way, two instances of datasets-server can run on the same namespace, with different domain names. Fixes #227 (comment)
* docs: ✏️ clarify doc about Release and Instance name * fix: 🐛 use the release name to name all the objects this way, mongo, which prefixes its name with the release, appears in the same "group" of pods when ordered alphabetically * fix: 🐛 remove "public," from cache-control it is not needed (public is only meant to force public caching for authenticated requests, which is not the case). And it prevented nginx to correctly cache the responses based on cache-control. * refactor: 💡 create one template directory per service Also: use a static name for the containers (see 7abd07d#r73243800) * feat: 🎸 ignore cpu and ram tests if MAX set to 0 Also: add doc to allow using HF_DATASETS_CACHE and HF_MODULES_CACHE to set the datasets library cache directories. * feat: 🎸 add NFS for /assets and dataset library /cache Also: remove the tests for the load (cpu/ram) in the workers. Note: we run the containers with user 1000 and group 3000, and use an initContainer to ensure the mounted volumes from NFS use these user and group. See #241 for a possible security improvement on this.
XciD
reviewed
May 11, 2022
infra/charts/datasets-server/templates/_initContainerAssets.tpl
Outdated
Show resolved
Hide resolved
XciD
reviewed
May 11, 2022
XciD
reviewed
May 11, 2022
infra/charts/datasets-server/templates/datasets-worker/deployment.yaml
Outdated
Show resolved
Hide resolved
XciD
reviewed
May 11, 2022
XciD
reviewed
May 11, 2022
|
Very nice job. LGTM |
see https://www.google.com/search?q=why+not+use+alpine for why not using alpine
because the length of the k8s names is limited
|
Thanks a lot for the review! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
see #223