(feat): Add Amazon Neptune support 🚀 #1085

bechbd · 2021-12-22T22:29:00Z

Issue #, if available:

Description of changes:
First draft of what a Neptune interface might look like.

I did have an utstanding question though on the naming of the write function names. There seems to be several conventions (put, to_sql, index, etc.) that different services have used based on how they work. Is there a preferred naming convention we would like to follow here?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

jaidisido · 2021-12-22T22:32:07Z

AWS CodeBuild CI Report

CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
Commit ID: f4f084b
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

jaidisido · 2021-12-22T23:02:50Z

AWS CodeBuild CI Report

CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
Commit ID: 5f3a307
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

kukushking · 2021-12-23T12:29:25Z

Regarding naming conventions we tend to just go with whatever makes sense for the domain / service really. I'm happy with to_graph and read_{gremlin, opencypher, sparql}.

kukushking · 2021-12-23T12:31:10Z

awswrangler/neptune/__init__.py

@@ -0,0 +1,17 @@
+"""Utilities Module for Amazon OpenSearch."""


Minor overlook there

kukushking · 2021-12-23T12:33:51Z

Also just note that there's convenient fix.sh in the root of the repo that you can run to sort imports and correct formatting. It basically just runs isort . and black . - this will get rid of static checking errors

jaidisido · 2021-12-23T19:11:36Z

AWS CodeBuild CI Report

CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
Commit ID: 6db725e
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

jaidisido

Thanks for this. Some general comments.

awswrangler/neptune/client.py

jaidisido · 2022-01-05T18:00:44Z

awswrangler/neptune/neptune.py

+    Run a Gremlin Query
+
+    >>> import awswrangler as wr
+    >>> client = wr.neptune.Client(host='NEPTUNE-ENDPOINT')


We don't tend to call a class directly in Wrangler. Instead perhaps we can replicate the pattern here where a connect method is called and the connection is then reused in other methods?

So for Neptune, I intended to use the HTTPS endpoints for all the languages, so there isn't really a long living connection like there is with some other databases. It's really just a client that has all the needed information to generate the appropriate requests call at the time of invocation. I was thinking that the client object here would be very similar to the connection object in Redshift. Thoughts?

The client can be implemented as a class, my comment was simply to indicate that no other method in the entire library is calling a class directly and we would prefer to keep it that way for UX. So it can be implemented as a class but there would be a helper to return an instance of the class like you mentioned for here

jaidisido · 2022-01-05T18:01:59Z

awswrangler/neptune/neptune.py

+
+    >>> import awswrangler as wr
+    >>> client = wr.neptune.Client(host='NEPTUNE-ENDPOINT')
+    >>> df = wr.neptune.gremlin.read(client, "g.V().limit(5).valueMap()")


I assume this should be wr.neptune.read_gremlin instead?

I would vote for wr.neptune.gremlin.read may be, because each graph might have their own functions and it creates a good isolation between property graph and RDF.

I went back and forth on this a bit and I ended up with read_gremlin and read_opencypher instead of creating subclasses for each language, as that seemed to better fit the overall patterns in the library. I must have missed that in the docs, but they have been updated now.

Agreed would prefer dedicated methods like above

jaidisido · 2022-01-05T18:03:56Z

awswrangler/neptune/neptune.py

+
+    >>> import awswrangler as wr
+    >>> client = wr.neptune.Client(host='NEPTUNE-ENDPOINT')
+    >>> wr.neptune.gremlin.to_graph(


Would there be one per graph type (i.e. gremlin, sparql) or should this be wr.neptune.to_graph?

After thinking about the feedback here, I ended up creating a to_property_graph() and to_rdf_graph() as the incoming data frame for each will have a different format. For Gremlin versus openCypher they will have the same format, so I decided to combine them.

That would be fine. Once we start implementing these methods it will become clearer if they should merged or not

A single method should be fine as long as input is a DataFrame but yes that would become clear when implementation is added

jaidisido · 2022-01-05T18:08:29Z

Also just note that there's convenient fix.sh in the root of the repo that you can run to sort imports and correct formatting. It basically just runs isort . and black . - this will get rid of static checking errors

To add to this, there is a validate.sh script that can be run locally or as a pre-commit hook for code static checks. An alternative is to prepend [skip ci] to your commit message which will simply skip the various Github checks we have

jaidisido · 2022-01-15T10:28:03Z

AWS CodeBuild CI Report

CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
Commit ID: b3c4e45
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

jaidisido · 2022-01-17T17:22:46Z

AWS CodeBuild CI Report

CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
Commit ID: 178130f
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…es and result parsing

…ociated with the three query languages

NickCorbett · 2022-01-26T01:35:03Z

awswrangler/neptune/client.py

+
+            # If the value is a Vertex or Edge do special processing
+            if isinstance(d[k], Vertex) or isinstance(d[k], Edge) or isinstance(d[k], VertexProperty) or isinstance(d[k], Property):
+                d[k] = d[k].__dict__


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

Modifying object.__dict__ directly or writing to an instance of a class __dict__ attribute directly is not recommended. Inside every module is a __dict__ object.dict attribute which contains its symbol table. If you modify object.__dict__, then the symbol table is changed. Also, direct assignment to the __dict__ attribute is not possible.

Learn more

NickCorbett · 2022-01-26T01:35:03Z

awswrangler/neptune/client.py

+
+        # For lists or paths unwind them
+        if isinstance(result, list) or isinstance(result, Path):
+            for x in result:


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

To create a list, try to use list comprehension instead of a loop. List comprehension is the preferred way to make a list using Python, and it's simpler and easier to understand than using a loop.

Learn more

NickCorbett · 2022-01-26T01:35:03Z

awswrangler/neptune/client.py

+        # If this is a list or Path then unwind it
+        if isinstance(data, list) or isinstance(data, Path):
+            res=[]        
+            for x in data:


Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

To create a list, try to use list comprehension instead of a loop. List comprehension is the preferred way to make a list using Python, and it's simpler and easier to understand than using a loop.

Learn more

kukushking · 2022-01-31T11:48:14Z

awswrangler/neptune/client.py

+
+    def read_gremlin(self, query, headers:Dict[str, Any] = None) -> Dict[str, Any]:
+        try:
+            nest_asyncio.apply()


I'm curious - is this required? Nesting event loops seems to be a controversial topic

I tried running it without nesting event loops and it does not work inside Jupyter without having it nested.

There should be a way to use a single event loop... let me check the details

So looks like they even added the ability to enable event loop nesting to the gremlin driver itself now. You can just enable it by passing transport_factory=AiohttpTransport(call_from_event_loop=True). I added a commit with this

…ty graphs

…s. Moved the Gremlin parsing code out of client and into its own static class

…o database infra scripting

…s well as added update option for property graph data

…tion pass locally

jaidisido · 2022-02-12T01:11:37Z

AWS CodeBuild CI Report

CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
Commit ID: 4257546
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

jaidisido · 2022-03-15T10:52:28Z

AWS CodeBuild CI Report

CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
Commit ID: 4aabf84
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

jaidisido · 2022-03-15T11:28:38Z

AWS CodeBuild CI Report

CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
Commit ID: 6f77ada
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

kukushking · 2022-03-15T17:01:07Z

awswrangler/neptune/client.py

+            uri = f"{HTTP_PROTOCOL}://{self.host}:{self.port}/gremlin"
+            request = self._prepare_request("GET", uri, headers=headers)
+            ws_url = f"{WS_PROTOCOL}://{self.host}:{self.port}/gremlin"
+            c = client.Client(


Just noticed this - do we need to open and close the connection on every query? Can this be moved to __init__?

jaidisido · 2022-03-15T17:21:03Z

AWS CodeBuild CI Report

CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
Commit ID: f06c01a
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

jaidisido · 2022-03-15T17:39:27Z

AWS CodeBuild CI Report

CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
Commit ID: 6ba9ccd
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…book for Amazon Neptune

into ISSUE-1084

jaidisido · 2022-03-18T20:46:22Z

AWS CodeBuild CI Report

CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
Commit ID: 462bc0b
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

jaidisido · 2022-03-21T14:44:18Z

AWS CodeBuild CI Report

CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
Commit ID: 8680416
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

jaidisido · 2022-03-21T15:18:00Z

AWS CodeBuild CI Report

CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
Commit ID: de1acf6
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

… name and added code to create the df when running cardinality tests.

into ISSUE-1084

jaidisido · 2022-03-21T15:51:57Z

AWS CodeBuild CI Report

CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
Commit ID: 49f5fdc
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

jaidisido · 2022-03-21T17:08:16Z

AWS CodeBuild CI Report

CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
Commit ID: 6ac2c6a
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Added first draft of potential interface for Neptune

f4f084b

Added first draft of potential interface for Neptune

5f3a307

kukushking linked an issue Dec 23, 2021 that may be closed by this pull request

Support for Amazon Neptune #1084

Closed

kukushking reviewed Dec 23, 2021

View reviewed changes

awswrangler/neptune/__init__.py Outdated

@@ -0,0 +1,17 @@

"""Utilities Module for Amazon OpenSearch."""

Copy link

Contributor

kukushking Dec 23, 2021

Choose a reason for hiding this comment

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

Minor overlook there

Fixed __init__.py file with the correct functions for Neptune

6db725e

jaidisido reviewed Jan 5, 2022

View reviewed changes

bechbd and others added 2 commits January 11, 2022 15:07

[skip ci] Updated signatures per initial feedback from draft PR

2a21a5e

Merge branch 'main' into ISSUE-1084

b3c4e45

Merge branch 'main' into ISSUE-1084

178130f

bechbd added 3 commits January 25, 2022 16:26

[skip ci] WIP - Initial version of oc and gremlin endpoint read queri…

41334b6

…es and result parsing

[skip ci] Initial working version of the basic read functionality ass…

355b8be

…ociated with the three query languages

[skip ci] Fixed tests that were not running correctly due to typo

ce3b697

NickCorbett reviewed Jan 26, 2022

View reviewed changes

kukushking reviewed Jan 31, 2022

View reviewed changes

bechbd added 6 commits February 2, 2022 12:04

WIP on writing data

2c84615

[skip ci] Have a working version of the complete roundtrip for proepr…

494e330

…ty graphs

[skip ci] Refactored code to simplify the locations into a utils clas…

b4531c2

…s. Moved the Gremlin parsing code out of client and into its own static class

[skip ci] Added SPARQL write functionality as well as added neptune t…

9df814c

…o database infra scripting

[skip ci] Readded nested asyncio in order for it to work in Jupyter a…

d768d85

…s well as added update option for property graph data

Working version of all MVP features as well as changes to make valida…

4257546

…tion pass locally

Added GremlinParser to the init file so that it can be unit tested.

31df062

jaidisido added 2 commits March 15, 2022 09:55

Validate fixes and security group in test_infra

201de73

Minor - typing and docs

4aabf84

jaidisido added 2 commits March 15, 2022 11:12

[skip ci] - Minor - Add Neptune docs entry

52ce870

Merge branch 'main' into ISSUE-1084

6f77ada

Use built-in gremlin driver functionality to enable event loop nesting

f06c01a

kukushking reviewed Mar 15, 2022

View reviewed changes

Use built-in gremlin driver functionality to enable event loop nesting

6ba9ccd

kukushking force-pushed the ISSUE-1084 branch from f06c01a to 6ba9ccd Compare March 15, 2022 17:25

bechbd added 3 commits March 18, 2022 10:17

Updated the connection handling for Gremlin and added a tutorial note…

6bf0450

…book for Amazon Neptune

Merge branch 'ISSUE-1084' of https://github.com/bechbd/aws-data-wrangler

71d273f

into ISSUE-1084

Fixed validation issues

462bc0b

bechbd marked this pull request as ready for review March 18, 2022 20:38

Minor - Remove nest_asyncio and fix tutorial

8680416

Upgrading dependencies

de1acf6

jaidisido changed the title ~~ISSUE-1084 - Added first draft of potential interface for Neptune~~ (feat): Add Amazon Neptune support 🚀 Mar 21, 2022

jaidisido and others added 3 commits March 21, 2022 15:39

Merge branch 'main' into ISSUE-1084

52dd72d

Fixed incomplete error message and updated tutorial to remove cluster…

f48830d

… name and added code to create the df when running cardinality tests.

Merge branch 'ISSUE-1084' of https://github.com/bechbd/aws-data-wrangler

49f5fdc

into ISSUE-1084

Minor - validate

6ac2c6a

jaidisido merged commit 65c2164 into aws:main Mar 21, 2022

jmahlik mentioned this pull request Apr 4, 2022

Neptune dependencies -- possibility of moving to optional/extras? #1252

Closed

		@@ -0,0 +1,17 @@
		"""Utilities Module for Amazon OpenSearch."""

(feat): Add Amazon Neptune support 🚀 #1085

(feat): Add Amazon Neptune support 🚀 #1085

Uh oh!

Conversation

bechbd commented Dec 22, 2021

Uh oh!

jaidisido commented Dec 22, 2021

AWS CodeBuild CI Report

Uh oh!

jaidisido commented Dec 22, 2021

AWS CodeBuild CI Report

Uh oh!

kukushking commented Dec 23, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kukushking commented Dec 23, 2021

Uh oh!

jaidisido commented Dec 23, 2021

AWS CodeBuild CI Report

Uh oh!

jaidisido left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaidisido Jan 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaidisido commented Jan 5, 2022

Uh oh!

jaidisido commented Jan 15, 2022

AWS CodeBuild CI Report

Uh oh!

jaidisido commented Jan 17, 2022

AWS CodeBuild CI Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaidisido commented Feb 12, 2022

AWS CodeBuild CI Report

Uh oh!

jaidisido commented Mar 15, 2022

AWS CodeBuild CI Report

Uh oh!

jaidisido commented Mar 15, 2022

AWS CodeBuild CI Report

jaidisido Jan 17, 2022 •

edited

Loading