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

fix: error when start gremlin-console with sample script #2231

Merged
merged 3 commits into from
Jun 12, 2023

Conversation

VGalaxies
Copy link
Contributor

@VGalaxies VGalaxies commented Jun 10, 2023

Purpose of the PR

Main Changes

  1. hugegraph-dist/src/assembly/static/bin/gremlin-console.sh
    1. update log4j configuration path
  2. hugegraph-dist/src/assembly/static/scripts/example.groovy
    1. add graph.serverStarted to set the scheduler information
    2. remove unnecessary index on properties [name]
    3. remove all semicolons
    4. modify hugegraph conf path

After modification, user should use the following command to start gremlin-console with sample script:

> ./bin/gremlin-console.sh -- -i scripts/example.groovy

         \,,,/
         (o o)
-----oOOo-(3)-oOOo-----
plugin activated: HugeGraph
plugin activated: tinkerpop.server
plugin activated: tinkerpop.utilities
plugin activated: tinkerpop.tinkergraph
main dict load finished, time elapsed 644 ms
model load finished, time elapsed 35 ms.
>>>> query all vertices: size=6
>>>> query all edges: size=6
gremlin> 

Currently, example.groovy can serve multiple purposes. The modification to example.groovy mentioned above is to adapt to the scenario where Gremlin-Console is started. If HugeGraphServer is started using scripts or IDEA, the initialization part of example.groovy should be:

conf = "conf/graphs/hugegraph.properties"
graph = HugeFactory.open(conf)
schema = graph.schema()

Verifying these changes

  • This change is a trivial rework / code cleanup without any test coverage.

Does this PR potentially affect the following parts?

  • Nope
  • Dependencies (add/update license info)
  • Modify configurations
  • The public API
  • Other affects (typed here)

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - NO Need

@imbajin imbajin changed the title fix: Error occurred when starting Gremlin-Console (stand-alone offline mode) with sample script fix: error when start gremlin-console with sample script Jun 10, 2023
graph = HugeFactory.open(conf);
schema = graph.schema();
graph = HugeFactory.open(conf)
graph.serverStarted(IdGenerator.of("server-tinkerpop"), NodeRole.MASTER)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zyxxoo we must start server with the node-role now?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, If it is started in a way that requires verification, then this is no longer needed. However, it’s okay to set it up. If verification is not required, then it still needs to be set up

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verification mean config
auth.authenticator= org.apache.hugegraph.auth.StandardAuthenticator

@@ -113,7 +113,7 @@ if [ -z "${HADOOP_GREMLIN_LIBS:-}" ]; then
fi

if [ -z "${JAVA_OPTIONS:-}" ]; then
JAVA_OPTIONS="-Dtinkerpop.ext=$EXT -Dlog4j.configuration=conf/log4j-console.properties -Dgremlin.log4j.level=$GREMLIN_LOG_LEVEL -javaagent:$LIB/jamm-0.3.0.jar"
JAVA_OPTIONS="-Dtinkerpop.ext=$EXT -Dlog4j.configurationFile=conf/log4j2.xml -Dgremlin.log4j.level=$GREMLIN_LOG_LEVEL -javaagent:$LIB/jamm-0.3.0.jar"
fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could also echo(print)the options if need(or helpful)

Copy link
Contributor Author

@VGalaxies VGalaxies Jun 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@imbajin If set -l TRACE or -l DEBUG, it will print out the actual running command with options.

./bin/gremlin-console.sh -l DEBUG -- -i scripts/example.groovy

@codecov
Copy link

codecov bot commented Jun 10, 2023

Codecov Report

Merging #2231 (9196664) into master (b8c7743) will increase coverage by 3.41%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #2231      +/-   ##
============================================
+ Coverage     65.06%   68.47%   +3.41%     
+ Complexity      979      977       -2     
============================================
  Files           498      498              
  Lines         40681    40681              
  Branches       5681     5681              
============================================
+ Hits          26468    27856    +1388     
+ Misses        11584    10114    -1470     
- Partials       2629     2711      +82     

see 67 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

javeme
javeme previously approved these changes Jun 10, 2023
import org.apache.hugegraph.dist.RegisterUtil
import org.apache.hugegraph.type.define.NodeRole
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tiny question: I didn't import this line before and it seems to work too, weird

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that some classes have been imported after plugin activated: HugeGraph.

> ./bin/gremlin-console.sh

         \,,,/
         (o o)
-----oOOo-(3)-oOOo-----
plugin activated: HugeGraph
plugin activated: tinkerpop.server
plugin activated: tinkerpop.utilities
plugin activated: tinkerpop.tinkergraph
gremlin> NodeRole
==>class org.apache.hugegraph.type.define.NodeRole

@javeme
Copy link
Contributor

javeme commented Jun 11, 2023

LGTM

Copy link
Member

@liuxiaocs7 liuxiaocs7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Member

@imbajin imbajin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc is also on the way, thanks for the improvement 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Bug] Error occurred when starting Gremlin-Console (stand-alone offline mode) with sample script
5 participants