Added compatibility with Neptune Analytics #241
Conversation
environment variable in node-server.js.
service type
node-server.js
draft implementation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #241 +/- ##
=======================================
Coverage 10.06% 10.06%
=======================================
Files 408 409 +1
Lines 30114 30150 +36
Branches 660 660
=======================================
+ Hits 3030 3036 +6
- Misses 26749 26779 +30
Partials 335 335
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -49,6 +49,7 @@ You can create and manage connections to graph databases using this feature. Con | |||
| - **Using proxy server:** Check this box if using a proxy endpoint. | |||
| - **Graph connection URL:** Provide the endpoint for the graph database | |||
| - **AWS IAM Auth Enabled:** Check this box if connecting to Amazon Neptune using IAM Auth and SigV4 signed requests | |||
| - **Service Type:** Choose the service type | |||
There was a problem hiding this comment.
Let's explain inline (so it is easier for folks to understand) - choose service time neptune-db for Neptune database, neptune-graph for Neptune Analytics or empty when not using IAM authentication. ?
| @@ -133,6 +135,7 @@ First, create a `config.json` file containing values for the connection attribut | |||
| "GRAPH_CONNECTION_URL": "https://cluster-cqmizgqgrsbf.us-west-2.neptune.amazonaws.com:8182", | |||
There was a problem hiding this comment.
| "GRAPH_CONNECTION_URL": "https://cluster-cqmizgqgrsbf.us-west-2.neptune.amazonaws.com:8182", | |
| "GRAPH_CONNECTION_URL": "https://cluster-somecluster.us-west-2.neptune.amazonaws.com:8182", |
| @@ -160,6 +163,7 @@ docker run -p 80:80 -p 443:443 \ | |||
| --env IAM=false \ | |||
| --env GRAPH_CONNECTION_URL=https://cluster-cqmizgqgrsbf.us-west-2.neptune.amazonaws.com:8182 \ | |||
There was a problem hiding this comment.
| --env GRAPH_CONNECTION_URL=https://cluster-cqmizgqgrsbf.us-west-2.neptune.amazonaws.com:8182 \ | |
| --env GRAPH_CONNECTION_URL=https://cluster-somecluster.us-west-2.neptune.amazonaws.com:8182 \ |
Exact endpoints should not be used in the documentation.
| @@ -79,6 +79,7 @@ const fetchVerticesAttributes = async ( | |||
| const response = await openCypherFetch<RawVerticesSchemaResponse>(verticesTemplate); | |||
|
|
|||
| const vertex = response.results[0]?.object as OCVertex; | |||
| if (!vertex) return; | |||
There was a problem hiding this comment.
nit: avoid adding a return statement, and wrap vertices.push({ within the condition, to avoid fragmented logic with multiple returns.
| if (!vertex) return; | |
| if (vertex) { | |
| }; |
There was a problem hiding this comment.
it's return early pattern
There was a problem hiding this comment.
I agree with @vkagamlyk in this case. Returning early when checking assertions is good practice.
The only nit I would have is to not use inline if statements. While it is more code, it is also more readable. It's much easier to find the places in a method that return early if the return call is in a predictable place. Having it inline makes the placement depend on the condition expression length.
if (!vertex) {
return;
}
packages/graph-explorer/src/modules/ConnectionDetail/ConnectionDetail.tsx
Show resolved
Hide resolved
packages/graph-explorer/src/modules/CreateConnection/CreateConnection.tsx
Show resolved
Hide resolved
packages/graph-explorer/src/modules/CreateConnection/CreateConnection.tsx
Outdated
Show resolved
Hide resolved
packages/graph-explorer/src/modules/CreateConnection/CreateConnection.tsx
Show resolved
Hide resolved
Co-authored-by: Alexey Temnikov <alexey.temnikov@improving.com>
vkagamlyk
dismissed stale reviews from michaelnchin and alexey-temnikov
via
d69e2bc
There was a problem hiding this comment.
Validated Connection + Graph UI functionality on the SageMaker-hosted app. Default connections require additional changes, but I will consider that outside the scope of this PR. Thanks @vkagamlyk !
This was covered in PR #254 |
Issue #, if available:
Description of changes:
Added option to connect to Neptune Analytics
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.