Athena to Amazon Neptune connector code #222

abhishekpradeepmishra · 2020-07-28T15:00:59Z

Issue #, if available:

Description of changes:
Athena to Amazon Neptune connector code

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

fixed issue with span function for null values

… endpoint

avirtuos · 2020-07-28T15:02:43Z

Thanks Abhishek, per our chime chat I'll get you some feedback tomorrow. Very excited that you and the team have been working on this.

avirtuos

Looks like a reasonable starting point. Most of my comments are easy to address. The most meaningful comment is about moving RecordHandler to use the extractor framework which is significantly faster (2x-4x) than the current structuring used.

athena-neptune/.factorypath

athena-neptune/.classpath

athena-neptune/.project

athena-neptune/.settings/org.eclipse.jdt.apt.core.prefs

athena-neptune/.settings/org.eclipse.jdt.core.prefs

athena-neptune/src/main/java/com/amazonaws/athena/connectors/neptune/NeptuneRecordHandler.java

athena-neptune/src/main/java/com/amazonaws/athena/connectors/neptune/TypeUtils.java

...eptune/src/test/java/com/amazonaws/athena/connectors/neptune/NeptuneMetadataHandlerTest.java

…Query Processor

avirtuos · 2020-09-03T14:01:12Z

Hey team, any updates here?

…nused references

abhishekpradeepmishra · 2020-09-22T20:04:51Z

Hey team, any updates here?

Done with code review comments and additional code related to mocking up RecordHandler Test . Sending new pull request

abhishekpradeepmishra · 2020-09-22T20:11:00Z

Sending new pull request

abhishekpradeepmishra · 2020-09-23T16:04:15Z

Re-opening as requested to have context of changes

2Fixed most of the checkstyle issues except for import ordr and wld crd

…federation into athena-neptune

avirtuos · 2020-10-01T19:59:32Z

@abhishekpradeepmishra Ill get you feedback in the next 24hrs, sorry for the delay.

avirtuos

We do not generally commit .gitignore in our projects. Can you remove this file?

athena-neptune/src/main/java/com/amazonaws/athena/connectors/neptune/NeptuneRecordHandler.java

athena-neptune/src/main/java/com/amazonaws/athena/connectors/neptune/TypeRowWriter.java

athena-neptune/athena-neptune.yaml

athena-neptune/src/main/java/com/amazonaws/athena/connectors/neptune/NeptuneRecordHandler.java

avirtuos · 2020-10-02T13:16:21Z

athena-neptune/src/main/java/com/amazonaws/athena/connectors/neptune/NeptuneRecordHandler.java

@@ -158,16 +157,20 @@ protected void readWithConstraint(final BlockSpiller spiller, final ReadRecordsR
                        final Map<Object, Object> obj = graphTraversalFinal.next();
                        return (rowWriter.writeRow(block, rowNum, (Object) obj) ? 1 : 0);
                    });
-                } catch (final Exception e) {
+                } 
+                catch (final Exception e) {
                    logger.info("readWithContraint: Exception occured " + e);


You need to re-throw this exception otherwise a customer using Athena will not see a useful error message. They might only see empty results and not even know there was an error.

Removed internal try catch block

avirtuos · 2020-10-02T13:17:29Z

athena-neptune/src/main/java/com/amazonaws/athena/connectors/neptune/NeptuneRecordHandler.java

@@ -158,16 +157,20 @@ protected void readWithConstraint(final BlockSpiller spiller, final ReadRecordsR
                        final Map<Object, Object> obj = graphTraversalFinal.next();
                        return (rowWriter.writeRow(block, rowNum, (Object) obj) ? 1 : 0);
                    });
-                } catch (final Exception e) {
+                } 
+                catch (final Exception e) {


Catching Exception is a bad practice, please catch only RuntimeException + any specific checked exceptions that the code requires (for example IOException is a specific exception that the compiler will complain about if code in your 'try' can throw it but you dont have a corresponding catch.

athena-neptune/src/main/java/com/amazonaws/athena/connectors/neptune/NeptuneRecordHandler.java

avirtuos · 2020-10-02T13:18:44Z

.gitignore

 .settings/org.eclipse.m2e.core.prefs
+.vscode/settings.json


We do not typically commit the .gitignore file to our project.

avirtuos · 2020-10-02T13:23:54Z

@abhishekpradeepmishra Great progress! I posted a few new comments but they are mostly minor. I resolved ~100 of the previously opened comments. Getting very close! Let me know what you are ready for another review.

Changes to SAR YAMl

…shra/aws-athena-query-federation into athena-neptune

avirtuos

looks good to me. I think this is ready to be merged. Thanks for working with us on this contribution.

avirtuos · 2020-10-14T18:34:47Z

Looks like there are a few checkstyle issues as you mentioned on IM. Let me know when you have these resolved and I can do a final review before merging. I'll also speak with the team to see when we can schedule final testing and deployment to serverless application repository on our side. @abhishekpradeepmishra

avirtuos · 2020-10-14T18:46:53Z

Do not merge yet, im discussing 1 issue with the team

… RecordSpill test case

avirtuos · 2020-10-15T13:54:28Z

@abhishekpradeepmishra and @sandeepveldi Thanks for this awesome contribution! I'll work with the team to get this into the next official release and also get a pre-built version into Serverless Application Repository.

abhishekpradeepmishra · 2020-10-15T15:09:18Z

cool, thanks for guidance

sandeepveldi · 2020-10-15T15:14:53Z

Thank you @avirtuos.

Initial merge of Athena's Neptune connector. Co-authored-by: Sandeep Veldi <veldi@amazon.com>

abhishekpradeepmishra and others added 11 commits July 1, 2020 22:43

fixed issue with span function for null values

6303507

Merge pull request #1 from abhishekpradeepmishra/dev

b68dcc3

fixed issue with span function for null values

Added code for Athena to Neptune connector

67fae55

added code to handle =, >=, <=

c351e44

Fixed unit test cases

cfd5c1c

added test base class, cleaned up NeptuneRecordHandler

e19fe66

Inherited from Testbase

4efbc44

refactored code to include TestBase and removed hardcoding of Neptune…

8eaf751

… endpoint

Corrected Readme and athena-neptune.yaml and deleted UDF classes

9520d2d

Refactored Athena Neptune Handler

b6c3277

refactored code, mocked test cases for MetaHandler

b1c39a3

avirtuos requested review from avirtuos and atennak1 July 28, 2020 15:04

avirtuos requested changes Aug 4, 2020

View reviewed changes

abhishekpradeepmishra added 4 commits August 12, 2020 14:24

implemented code review comments

7b87f27

removed unwanted files

f8c184a

mocked record handler code, removed string transformation in Gremlin …

a246be6

…Query Processor

removed string templates

7619577

Sandeep Veldi and others added 4 commits September 15, 2020 23:03

Added eclipse prefs to git ignore list

23fe7bf

implemented RowWriter pattern, code clean-up and refactoring pending

a810b84

added more contraints to RecordHanderTest, refactored code, removed u…

9ef2e2a

…nused references

refactored code

e30ee4f

abhishekpradeepmishra closed this Sep 22, 2020

abhishekpradeepmishra reopened this Sep 23, 2020

1Modified SAM yaml to separate DB permissions

60712ab

2Fixed most of the checkstyle issues except for import ordr and wld crd

Sandeep Veldi added 2 commits September 23, 2020 22:57

Merge branch 'master' of https://github.com/awslabs/aws-athena-query-…

0e408f5

…federation into athena-neptune

Modifications align with latest FederatedIdentity

9369a6f

avirtuos requested changes Oct 2, 2020

View reviewed changes

avirtuos reviewed Oct 2, 2020

View reviewed changes

avirtuos self-assigned this Oct 2, 2020

abhishekpradeepmishra and others added 8 commits October 6, 2020 15:03

code review changes

e4d654f

Draft version of Readme file

ca4b7d7

Changes to SAR YAMl

Merge branch 'athena-neptune' of https://github.com/abhishekpradeepmi…

d0cfa3e

…shra/aws-athena-query-federation into athena-neptune

code review related changes

33be79b

Merge remote-tracking branch 'upstream/master' into athena-neptune

73998cb

Updated data types in readme and updated gitignore

56f4fb9

Merge remote-tracking branch 'upstream/master' into athena-neptune

e98adb3

added support for more data types

7fc923c

avirtuos previously approved these changes Oct 14, 2020

View reviewed changes

Fixed import order formatting issue.

0de38d3

sandeepveldi dismissed avirtuos’s stale review via 0de38d3 October 14, 2020 18:36

abhishekpradeepmishra added 2 commits October 15, 2020 02:20

added test cases constraints for Boolean and BigInt, fixed issue with…

8ff2c50

… RecordSpill test case

committing chagnes for test cases

05cf0c7

avirtuos approved these changes Oct 15, 2020

View reviewed changes

avirtuos merged commit b178e51 into awslabs:master Oct 15, 2020

abhishekpradeepmishra added a commit to abhishekpradeepmishra/aws-athena-query-federation that referenced this pull request Oct 15, 2020

Athena to Amazon Neptune connector code (awslabs#222)

30fe584

Initial merge of Athena's Neptune connector. Co-authored-by: Sandeep Veldi <veldi@amazon.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Athena to Amazon Neptune connector code #222

Athena to Amazon Neptune connector code #222

abhishekpradeepmishra commented Jul 28, 2020

avirtuos commented Jul 28, 2020

avirtuos left a comment

avirtuos commented Sep 3, 2020

abhishekpradeepmishra commented Sep 22, 2020

abhishekpradeepmishra commented Sep 22, 2020

abhishekpradeepmishra commented Sep 23, 2020

avirtuos commented Oct 1, 2020

avirtuos left a comment

avirtuos Oct 2, 2020

abhishekpradeepmishra Oct 6, 2020

avirtuos Oct 2, 2020

sandeepveldi Oct 14, 2020

avirtuos Oct 2, 2020

avirtuos commented Oct 2, 2020

avirtuos left a comment

avirtuos commented Oct 14, 2020

avirtuos commented Oct 14, 2020

avirtuos commented Oct 15, 2020

abhishekpradeepmishra commented Oct 15, 2020

sandeepveldi commented Oct 15, 2020

		.settings/org.eclipse.m2e.core.prefs
		.vscode/settings.json

Athena to Amazon Neptune connector code #222

Athena to Amazon Neptune connector code #222

Conversation

abhishekpradeepmishra commented Jul 28, 2020

avirtuos commented Jul 28, 2020

avirtuos left a comment

Choose a reason for hiding this comment

avirtuos commented Sep 3, 2020

abhishekpradeepmishra commented Sep 22, 2020

abhishekpradeepmishra commented Sep 22, 2020

abhishekpradeepmishra commented Sep 23, 2020

avirtuos commented Oct 1, 2020

avirtuos left a comment

Choose a reason for hiding this comment

avirtuos Oct 2, 2020

Choose a reason for hiding this comment

abhishekpradeepmishra Oct 6, 2020

Choose a reason for hiding this comment

avirtuos Oct 2, 2020

Choose a reason for hiding this comment

sandeepveldi Oct 14, 2020

Choose a reason for hiding this comment

avirtuos Oct 2, 2020

Choose a reason for hiding this comment

avirtuos commented Oct 2, 2020

avirtuos left a comment

Choose a reason for hiding this comment

avirtuos commented Oct 14, 2020

avirtuos commented Oct 14, 2020

avirtuos commented Oct 15, 2020

abhishekpradeepmishra commented Oct 15, 2020

sandeepveldi commented Oct 15, 2020