-
Notifications
You must be signed in to change notification settings - Fork 521
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
feat: support Cassandra with docker-compose in server #2307
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2307 +/- ##
============================================
+ Coverage 63.85% 68.29% +4.44%
- Complexity 684 987 +303
============================================
Files 505 505
Lines 41902 41902
Branches 5817 5817
============================================
+ Hits 26756 28619 +1863
+ Misses 12455 10513 -1942
- Partials 2691 2770 +79 see 94 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
.gitignore
Outdated
@@ -83,3 +83,5 @@ hs_err_pid* | |||
.mtj.tmp/ | |||
# blueJ files | |||
*.ctxt | |||
|
|||
*swagger-ui* |
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.
seems no need to add here due to the master code has fixed it (#2277)
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.
Dockerfile
Outdated
|
||
EXPOSE 8080 | ||
VOLUME /hugegraph | ||
|
||
ENTRYPOINT ["/usr/bin/dumb-init", "--"] | ||
CMD ["./bin/start-hugegraph.sh", "-d false -j $JAVA_OPTS -g zgc"] | ||
CMD ["./bin/docker-entrypoint.sh"] |
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.
maybe we shouldn't put the docker
files in general path?
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.
https://github.com/JanusGraph/janusgraph/tree/master/janusgraph-dist/docker
maybe I can try to move the docker related file in a split folder in hugegraph-dist, like janusgraph?
|
||
./bin/init-store.sh | ||
|
||
./bin/start-hugegraph.sh -d false -j "$JAVA_OPTS" -g zgc |
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.
why the params here? could we get the value of $JAVA_OPTS
?
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.
JAVA_OPTS is an env var which is set in dockerfile. And this shell is only used in dockerfile, hence I think this way is available.
depends_on: | ||
- cassandra | ||
healthcheck: | ||
test: ["CMD", "bin/gremlin-console.sh", "--" ,"-e", "scripts/remote-connect.groovy"] |
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.
we still need -e
?
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.
https://tinkerpop.apache.org/docs/3.6.2-SNAPSHOT/reference/#execution-mode
-e
is for the gremlin console to use the execution mode to execute scripts.
retries: 3 | ||
|
||
cassandra: | ||
image: cassandra:3.11 |
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.
try C* 4 for it
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.
RegisterUtil.registerPalo() | ||
RegisterUtil.registerPostgresql() | ||
|
||
graph = HugeFactory.open('./conf/graphs/hugegraph.properties') |
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 rename this file to detect-storage.groovy
?
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.
Sure. And "detect" seems better than "try". I have applied the change.
TOP="$(cd "$BIN"/../ && pwd)" | ||
GRAPH_PROP="$TOP/conf/graphs/hugegraph.properties" | ||
HUGE_STORAGE_TIMEOUT_S=120 | ||
TRY_STORAGE="$TOP/scripts/detect-storage.groovy" |
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.
also update TRY_STORAGE
BIN=$(abs_path) | ||
TOP="$(cd "$BIN"/../ && pwd)" | ||
GRAPH_PROP="$TOP/conf/graphs/hugegraph.properties" | ||
HUGE_STORAGE_TIMEOUT_S=120 |
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.
prefer WAIT_STORAGE_TIMEOUT_S
|
||
BIN=$(abs_path) | ||
TOP="$(cd "$BIN"/../ && pwd)" | ||
GRAPH_PROP="$TOP/conf/graphs/hugegraph.properties" |
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.
prefer GRAPH_CONF
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.
The changes have been applied on the var name
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.
LGTM
1. change the dockerfile, adding the shell to wait for storage backend and use a docker-entrypoint.sh to manage the starting process. 2. delete a deprecated class in gremlin-console.sh (reference: [doc of ScriptExecutor](https://tinkerpop.apache.org/javadocs/3.2.3/full/org/apache/tinkerpop/gremlin/groovy/jsr223/ScriptExecutor.html)) 3. add a healthy check in docker-compose 4. add an example folder where we can put all the template docker-compose.yml here 5. add `*swagger-ui*` in gitignore, which appears after you compile the source code locally. --------- Co-authored-by: imbajin <[email protected]>
1. change the dockerfile, adding the shell to wait for storage backend and use a docker-entrypoint.sh to manage the starting process. 2. delete a deprecated class in gremlin-console.sh (reference: [doc of ScriptExecutor](https://tinkerpop.apache.org/javadocs/3.2.3/full/org/apache/tinkerpop/gremlin/groovy/jsr223/ScriptExecutor.html)) 3. add a healthy check in docker-compose 4. add an example folder where we can put all the template docker-compose.yml here 5. add `*swagger-ui*` in gitignore, which appears after you compile the source code locally. --------- Co-authored-by: imbajin <[email protected]>
…840)
Purpose of the PR
Main Changes
*swagger-ui*
in gitignore, which appears after you compile the source code locally.Detailed doc of the pr is here: https://hugegraph.feishu.cn/wiki/BBrgwmytNi0DiQkvDj1cpNbFn5b
Some test case: https://hugegraph.feishu.cn/docx/CBcydSr6CokDpexlZAQcH4Icnlb#TbYrdpl3io1c9NxT4vLcDTn8nhg
Verifying these changes
Does this PR potentially affect the following parts?
Documentation Status
Doc - TODO
Doc - Done
Doc - No Need