Parameterized k8s volume mount in session config #100
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
Codecov Report
@@ Coverage Diff @@
## main #100 +/- ##
==========================================
+ Coverage 78.05% 78.07% +0.02%
==========================================
Files 50 50
Lines 4511 4529 +18
==========================================
+ Hits 3521 3536 +15
- Misses 990 993 +3
Continue to review full report at Codecov.
|
sighingnow
approved these changes
Jan 22, 2021
| @@ -33,7 +33,7 @@ function _start { | |||
| sleep 5s | |||
| curl -XPOST http://localhost:${_port} -d 'import graphscope' | |||
| curl -XPOST http://localhost:${_port} -d 'from graphscope.framework.loader import Loader' | |||
| curl_sess="curl -XPOST http://localhost:${_port} -d 'session = graphscope.session(num_workers=${workers}, show_log=True, k8s_coordinator_cpu=1.0, k8s_coordinator_mem='\''4Gi'\'', k8s_vineyard_cpu=1.0, k8s_vineyard_mem='\''4Gi'\'', k8s_vineyard_shared_mem='\''4Gi'\'', k8s_engine_cpu=1.0, k8s_engine_mem='\''4Gi'\'', k8s_gie_graph_manager_image='\''${gie_manager_image}'\'', k8s_gs_image='\''${gs_image}'\'')' --write-out %{http_code} --silent --output ./curl.tmp" | |||
| curl_sess="curl -XPOST http://localhost:${_port} -d 'session = graphscope.session(num_workers=${workers}, show_log=True, k8s_volumes={\"data\": {\"type\": \"hostPath\", \"field\": {\"path\": \"${GS_TEST_DIR}\", \"type\": \"Directory\"}, \"mounts\": {\"mountPath\": \"/testingdata\"}}}, k8s_coordinator_cpu=1.0, k8s_coordinator_mem='\''4Gi'\'', k8s_vineyard_cpu=1.0, k8s_vineyard_mem='\''4Gi'\'', k8s_vineyard_shared_mem='\''4Gi'\'', k8s_engine_cpu=1.0, k8s_engine_mem='\''4Gi'\'', k8s_gie_graph_manager_image='\''${gie_manager_image}'\'', k8s_gs_image='\''${gs_image}'\'')' --write-out %{http_code} --silent --output ./curl.tmp" | |||
There was a problem hiding this comment.
Looks that we still need GS_TEST_DIR, but we delete that in scripts/test.sh. Is that desired?
There was a problem hiding this comment.
Yes, already remove all hard code GS_TEST_DIR in python code.
| } | ||
| ] | ||
| } | ||
| } |
There was a problem hiding this comment.
"Only 'hostPath' supported yet." -> "Only 'hostPath' is supported."
Add more about the docstring:
- If we just want to mount a single volume, could we write:
"mounts": {"mountPath": "<path1>"}? - will we resolve the environment varaibles if
$exists in the "path" or "mountPath"?
There was a problem hiding this comment.
- Yes. you can. I have updated the doc.
- Thanks for your advice. I think there is some confusion for users if we support parsing env variables because it occurs in coordinator pod, instead of python client. If we do that, we also have to support passing env in session params.
| "--k8s_volumes", | ||
| type=str, | ||
| default="{}", | ||
| help="A str of dict for kubernetes volumes.", |
There was a problem hiding this comment.
"A str of dict" -> "A json string"
There was a problem hiding this comment.
Thanks a lot.
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.
Fixes #47