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

Support to create graph dynamically #1065

Merged
merged 15 commits into from
Dec 24, 2021
Merged

Support to create graph dynamically #1065

merged 15 commits into from
Dec 24, 2021

Conversation

Linary
Copy link
Contributor

@Linary Linary commented Jun 30, 2020

Implement #165

Change-Id: I03d3e606d6292acec46a7725094c05fe84a6c9cd

@@ -69,6 +78,17 @@ public void start() throws IOException {
this.calcMaxWriteThreads();
}

private void fillGraphsOption(String graphsDir) {
Copy link
Contributor

Choose a reason for hiding this comment

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

move to graph manager

import com.baidu.hugegraph.util.E;
import com.baidu.hugegraph.util.Log;

public class InitStore {

private static final Logger LOG = Log.logger(InitStore.class);

private static final String GRAPHS = ServerOptions.GRAPHS.name();
private static final String GRAPHS = "graphs";
Copy link
Contributor

Choose a reason for hiding this comment

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

still keep it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

move to ConfigUtil

@@ -88,6 +96,17 @@ public void loadGraphs(final Map<String, String> graphConfs) {
}
}

public HugeGraph createGraph(HugeConfig config) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ensure auth permission: write graph

@Timed
@Path("{name}")
@Produces(APPLICATION_JSON_WITH_CHARSET)
@RolesAllowed({"admin"})
Copy link
Contributor

Choose a reason for hiding this comment

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

keep owner

@Linary Linary force-pushed the dynamic-add-graph branch 5 times, most recently from eb6230e to 80ad81d Compare July 2, 2020 10:16

HugeGraphServer server = new HugeGraphServer(args[0], args[1]);
Runtime.getRuntime().addShutdownHook(new Thread(() -> {
LOG.info("HugeGraphServer stopping");
server.stop();
LOG.info("HugeGraphServer stopped");
Copy link
Contributor

Choose a reason for hiding this comment

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

sure to remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure, just feel the logs are a bit redundant

@javeme
Copy link
Contributor

javeme commented Jul 16, 2020

api test error:

# Set backend and serializer
sed -i "s/backend=.*/backend=$BACKEND/" $CONF
sed: can't read hugegraph-0.11.1/conf/hugegraph.properties: No such file or directory
cat: hugegraph-0.11.1/logs/hugegraph-server.log: No such file or directory
The command "$TRAVIS_DIR/run-api-test.sh" exited with 1.
312.45s$ $TRAVIS_DIR/run-unit-test.sh

@javeme
Copy link
Contributor

javeme commented Jul 16, 2020

cassandra:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.20:test (core-test) on project hugegraph-test: There are test failures.
[ERROR] 
[ERROR] Please refer to /home/travis/build/hugegraph/hugegraph/hugegraph-test/target/surefire-reports for the individual test results.
[ERROR] Please refer to dump files (if any exist) [date]-jvmRun[N].dump, [date].dumpstream and [date]-jvmRun[N].dumpstream.
[ERROR] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :hugegraph-test
The command "mvn test -P core-test,$BACKEND" exited with 1.

postgre:

[ERROR] Failed to execute goal org.jacoco:jacoco-maven-plugin:0.8.4:dump (pull-test-data) on project hugegraph-test: Unable to dump coverage data: Connection refused (Connection refused) -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
The command "$TRAVIS_DIR/run-api-test.sh" exited with 1.

@codecov
Copy link

codecov bot commented Aug 11, 2020

Codecov Report

Merging #1065 (24972bb) into master (40a85ed) will decrease coverage by 0.14%.
The diff coverage is 30.43%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1065      +/-   ##
============================================
- Coverage     66.77%   66.62%   -0.15%     
- Complexity     7063     7092      +29     
============================================
  Files           420      423       +3     
  Lines         34726    34953     +227     
  Branches       4822     4844      +22     
============================================
+ Hits          23187    23289     +102     
- Misses         9196     9307     +111     
- Partials       2343     2357      +14     
Impacted Files Coverage Δ
...src/main/java/com/baidu/hugegraph/HugeFactory.java 70.37% <0.00%> (-5.63%) ⬇️
...java/com/baidu/hugegraph/type/define/NodeRole.java 68.75% <ø> (ø)
...src/main/java/com/baidu/hugegraph/util/Events.java 0.00% <ø> (ø)
...c/main/java/com/baidu/hugegraph/cmd/InitStore.java 0.00% <0.00%> (ø)
...ava/com/baidu/hugegraph/api/profile/GraphsAPI.java 10.44% <7.69%> (-0.27%) ⬇️
...in/java/com/baidu/hugegraph/core/GraphManager.java 37.16% <12.08%> (-14.56%) ⬇️
...com/baidu/hugegraph/auth/ContextGremlinServer.java 32.25% <21.05%> (-21.59%) ⬇️
...main/java/com/baidu/hugegraph/util/ConfigUtil.java 48.78% <48.78%> (ø)
.../com/baidu/hugegraph/server/ApplicationConfig.java 91.48% <71.42%> (-1.99%) ⬇️
...va/com/baidu/hugegraph/dist/HugeGremlinServer.java 52.38% <75.00%> (+20.95%) ⬆️
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 40a85ed...24972bb. Read the comment docs.

// but it's not easy to check here
String backend = config.get(CoreOptions.BACKEND);
if (backend.equalsIgnoreCase("rocksdb")) {
// TODO: should check data path...
Copy link
Contributor

Choose a reason for hiding this comment

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

seem same data path is ok

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same data path is not ok

"The graph name '%s' has existed", name);

PropertiesConfiguration propConfig = this.buildConfig(configText);
HugeConfig config = new HugeConfig(propConfig, false);
Copy link

Choose a reason for hiding this comment

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

Cannot resolve constructor 'HugeConfig(org.apache.commons.configuration.PropertiesConfiguration, boolean);
are you sure? @ @javeme @Linary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This constructor was defined in hugegraph-common 1.7.10, it's not yet published to the maven central repository.

@dyhyao6
Copy link

dyhyao6 commented Nov 5, 2020

能不能加快实现图的热更新,thanks @Linary @javeme

@@ -57,8 +57,8 @@ check_port "$REST_SERVER_URL"

echo "Starting HugeGraphServer..."

"$BIN"/hugegraph-server.sh "$CONF"/gremlin-server.yaml "$CONF"/rest-server.properties \
"$OPEN_SECURITY_CHECK" "$USER_OPTION" "$GC_OPTION" >>"$LOGS/hugegraph-server.log" 2>&1 &
${BIN}/hugegraph-server.sh ${CONF}/gremlin-server.yaml ${CONF}/rest-server.properties \
Copy link
Contributor

Choose a reason for hiding this comment

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

mkdir -p ${LOGS}

@github-actions
Copy link

Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label

@github-actions github-actions bot removed the inactive label Oct 9, 2021
Linary and others added 9 commits December 22, 2021 10:58
Implement #165

Change-Id: I03d3e606d6292acec46a7725094c05fe84a6c9cd
Change-Id: I5626be6a0c56074c51339283bf6f6e40914ae56b
Change-Id: I3816cdd3f2ab41f8976256956dd7dbb5fa6bab89
Change-Id: I276bf797a7cd601aaf6c9de1a5148cf05fba9b7d
Change-Id: I0a7f08454be5313478ecc0596731a6bc5f7dec98
Change-Id: I36e8815dd7d1ecac5615dd6a8638ff5ad13414d2
Change-Id: I57ca9ed3db38c77189714840de24b6c5e5e78ad1
Change-Id: Iedc438f6dee08e48dce2868e69555aaf0c058401
Change-Id: I08898184d6bc0323876dfb4a7398b7fafd43f8db
Change-Id: I37a92977c33332e5e8ab9f2b502c2283aa6b0343
javeme
javeme previously approved these changes Dec 22, 2021
@Consumes(MediaType.TEXT_PLAIN)
@Produces(APPLICATION_JSON_WITH_CHARSET)
@RolesAllowed({"admin", "$owner=$name"})
public Object manage(@Context GraphManager manager,
Copy link
Contributor

Choose a reason for hiding this comment

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

add delete method and move clone to create

Change-Id: I4c0220a2f22163037449a6cb829756ad78f7b942
Change-Id: I3322cd9be94b9137ed96e5d8f9211af7f54f926f
LOG.debug("Create graph {} with config options '{}'", name, configText);
HugeGraph graph = manager.createGraph(name, configText);
HugeGraph graph;
if (clone != null && clone.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should be "!clone.isEmpty()"?

Change-Id: I87dd6412b280f3c04ba5a405bb4d9013270adf3d
Change-Id: I4a9b25d4a2a5b0f722cb0f46b5a93f28e4ccccb2
javeme
javeme previously approved these changes Dec 24, 2021
return graph;
}

private PropertiesConfiguration buildConfig(String configText) {
Copy link
Contributor

Choose a reason for hiding this comment

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

check configText not null? and seems empty string is ok

Change-Id: If328e38979f7ee65b745fe52fcbf7c9c76b24df1
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.

test in client side

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

Successfully merging this pull request may close these issues.

None yet

5 participants