-
Notifications
You must be signed in to change notification settings - Fork 295
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
Updated SigV4 signing library in Gremlin connection for Neptune connector #1698
Conversation
This PR also provides a fix to the custom schema writer that allows a workaround for issue #1655, where naming clashes can be avoided by creating a view from custom queries. |
50ba5b8
to
51400a7
Compare
value.value = Float.parseFloat(fieldValue.toString()); | ||
value.isSet = 1; | ||
} | ||
else { | ||
else if (fieldValue instanceof ArrayList) { | ||
ArrayList<Object> objValues = (ArrayList) fieldValue; | ||
if (objValues != null && objValues.get(0) != null && !(objValues.get(0).toString().trim().isEmpty())) { | ||
value.value = Float.parseFloat(objValues.get(0).toString()); |
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.
Shouldn't we parse the list of value instead? Why are we parsing only the first element in list
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.
We can only return zero or one value for a column. Custom query may return a single value wrapped inside a list/set, so we needed to access the element. In corner cases multiple values are returned, we are also only returning the first value based on this constraint.
|
||
if (fieldValue != null) { | ||
if (fieldValue.getClass().equals(String.class)) { | ||
value.value = fieldValue.toString(); | ||
value.isSet = 1; | ||
} | ||
else { | ||
else if (fieldValue instanceof ArrayList) { |
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.
Do we need to apply this condition else if (fieldValue instanceof ArrayList)
to rest of types? For example DATEMILLI
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.
Yes, I did miss these. I'll add them.
b48cb2f
to
f7b2e1d
Compare
…r to Neptune supported version
f7b2e1d
to
ba27ee8
Compare
ba27ee8
to
3541598
Compare
@chngpe - Gentle ping to see if we can get this reviewed and merged soon. This is blocking a few customers as the current version of the connector does not work with an IAM-enabled Neptune cluster. |
Hi @chngpe, is there an ETA for merging this PR? Or is there anything else I should update? Thanks! |
@xiazcy done; thank you for your contribution. |
- Revert "build(deps): bump com.google.cloud:google-cloud-bigquery" (#1971) - build(deps): bump com.google.cloud:google-cloud-bigquery from 2.37.0 to 2.40.1 (#1962) - build(deps): bump com.google.cloud:google-cloud-bigquery - build(deps): bump com.google.api.grpc:grpc-google-cloud-bigquerystorage-v1 from 2.47.0 to 3.5.1 (#1961) - build(deps): bump com.google.api.grpc:grpc-google-cloud-bigquerystorage-v1 - build(deps): bump gremlinDriverVersion from 3.6.5 to 3.7.2 (#1967) - build(deps): bump gremlinDriverVersion from 3.6.5 to 3.7.2 - build(deps): bump com.google.cloud:google-cloud-resourcemanager from 1.44.0 to 1.45.0 (#1968) - build(deps): bump com.google.cloud:google-cloud-resourcemanager - build(deps): bump org.elasticsearch.client:elasticsearch-rest-client from 8.13.3 to 8.13.4 (#1970) - build(deps): bump org.elasticsearch.client:elasticsearch-rest-client - build(deps): bump com.amazon.redshift:redshift-jdbc42 from 2.1.0.26 to 2.1.0.27 (#1969) - build(deps): bump com.amazon.redshift:redshift-jdbc42 - build(deps): bump software.amazon.awssdk:bom from 2.25.45 to 2.25.50 (#1965) - build(deps): bump software.amazon.awssdk:bom from 2.25.45 to 2.25.50 - build(deps): bump com.google.cloud:google-cloud-storage from 2.37.0 to 2.38.0 (#1966) - build(deps): bump com.google.cloud:google-cloud-storage - build(deps): bump aws-sdk.version from 1.12.715 to 1.12.720 (#1964) - build(deps): bump aws-sdk.version from 1.12.715 to 1.12.720 - Enable map of complex types support (#1917) - unifying vertica connector as jdbc connector (#1918) - Feature/oracle fips (#1946) - Updated SigV4 signing library in Gremlin connection for Neptune connector (#1698)
- Snowflake split issue (#1963) - Com.google.cloud google cloud bigquery version update (#1975) - Fix ValidationException for Timestream connector (#1888) - build(deps): bump com.amazon.redshift:redshift-jdbc42 from 2.1.0.27 to 2.1.0.28 in /athena-redshift (#1974) - build(deps): bump com.amazon.redshift:redshift-jdbc42 - Updated prepare_dev_env.sh script (#1972) - Revert "build(deps): bump com.google.cloud:google-cloud-bigquery" (#1971) - build(deps): bump com.google.cloud:google-cloud-bigquery from 2.37.0 to 2.40.1 (#1962) - build(deps): bump com.google.cloud:google-cloud-bigquery - build(deps): bump com.google.api.grpc:grpc-google-cloud-bigquerystorage-v1 from 2.47.0 to 3.5.1 (#1961) - build(deps): bump com.google.api.grpc:grpc-google-cloud-bigquerystorage-v1 - build(deps): bump gremlinDriverVersion from 3.6.5 to 3.7.2 (#1967) - build(deps): bump gremlinDriverVersion from 3.6.5 to 3.7.2 - build(deps): bump com.google.cloud:google-cloud-resourcemanager from 1.44.0 to 1.45.0 (#1968) - build(deps): bump com.google.cloud:google-cloud-resourcemanager - build(deps): bump org.elasticsearch.client:elasticsearch-rest-client from 8.13.3 to 8.13.4 (#1970) - build(deps): bump org.elasticsearch.client:elasticsearch-rest-client - build(deps): bump com.amazon.redshift:redshift-jdbc42 from 2.1.0.26 to 2.1.0.27 (#1969) - build(deps): bump com.amazon.redshift:redshift-jdbc42 - build(deps): bump software.amazon.awssdk:bom from 2.25.45 to 2.25.50 (#1965) - build(deps): bump software.amazon.awssdk:bom from 2.25.45 to 2.25.50 - build(deps): bump com.google.cloud:google-cloud-storage from 2.37.0 to 2.38.0 (#1966) - build(deps): bump com.google.cloud:google-cloud-storage - build(deps): bump aws-sdk.version from 1.12.715 to 1.12.720 (#1964) - build(deps): bump aws-sdk.version from 1.12.715 to 1.12.720 - Enable map of complex types support (#1917) - unifying vertica connector as jdbc connector (#1918) - Feature/oracle fips (#1946) - Updated SigV4 signing library in Gremlin connection for Neptune connector (#1698)
Issue #, if available:
#1574
Description of changes:
The SigV4 library used for IAM in Gremlin connection in Neptune connector is a deprecated library, this PR updates it to use the new maintained library for signing. The deprecated library is only recommended to be used with 3.4.x Gremlin drivers, and may work when using 3.5.x and earlier 3.6.x driver versions, but will no longer work with 3.7.x.
Also downgraded the Gremlin driver version used by the connector to within the supported range for Neptune (currently 3.6.5 is the max).
I was able to reproduce the error on 3.7.1 gremlin console, trying to connect to IAM enabled cluster using the old SigV4 signing library:
I have tested the fix in this PR by deploying a customer connector and querying an IAM enabled cluster, and was successfull:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.