-
Notifications
You must be signed in to change notification settings - Fork 4
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
Integration with Neptune Analytics #31
Merged
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
…er for analytics vs neptune db.
…gic (#5) Introduce usage of the neptune graph (analytics) SDK in a couple scenarios: 1. during pipeline creation, if an axios request fails when querying the Neptune Analytics graph and falls back to SDK and 2. if the user has opted to use SDK for the generated lambda (as opposed to http). Summary of changes: -added new dependency on client-neptune-graph version 3.662.0 -created new lambda template which is used if the user specifies --output-resolver-query-sdk option -set additional lambda environment variable for neptune db name which is required to execute queries using the neptune graph SDK -added logic to fall back to neptune graph SDK if Axios request fails during pipeline creation (previous logic threw Error as the analytics SDK was not yet available) -fixed function which retrieves graph summary to use neptune graph SDK if the neptune-type is neptune-graph (the summary endpoint path for neptune-db is not the same for neptune-graph) -fixed CDK pipeline to only fetch cluster info if the type is neptune-db as it is not required for Neptune-graph (analytics) -set isNeptuneIAMAuth to true if the neptune type is detected as neptune-graph -introduced util.js for parsing functions that are used across multiple modules -refactored function which had many params to use an object param instead for better readability -introduced new test case 7 which sets --output-resolver-query-sdk option
…equired for axios credential interceptor in the http lambda.
I updated the formatting of the PR description. I hope that was ok. |
kmcginnes
reviewed
Oct 14, 2024
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.
Mostly looks good. Just a few comments.
…nsole error messages to reference neptune-type. Changed logic which determines neptune-type from endpoint to examine the host only.
…cessed so that it is not created if there are errors with the input. Changed lambda action param to be array of single item. Changed http lambda to use default service name of neptune-db if the environment variable is not set.
kmcginnes
reviewed
Oct 15, 2024
kmcginnes
approved these changes
Oct 15, 2024
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.
Looks good to me.
… params. Added default region back and added logic to switch to parsed region from endpoint if they differ. Added some comments for clarity around parsing of neptune analytics graph type. Changed CDK template to inline function from util.js instead of require or import. Refactor node property and edge name collision to use a set and added unit test.
andreachild
force-pushed
the
achild/anSdk
branch
from
October 15, 2024 22:43
6fda926
to
46d6c1f
Compare
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.
Added support for integration with Neptune Analytics via SDK or http:
client-neptune-graph
--output-resolver-query-sdk
optionneptune-type
isneptune-graph
(the summary endpoint path forneptune-db
is not the same forneptune-graph
)neptune-db
as it is not required forneptune-graph
(analytics)isNeptuneIAMAuth
to true if the neptune type is detected asneptune-graph
util.js
for parsing functions that are used across multiple modules--output-resolver-query-sdk
option