-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
🎉 Created destination-oracle-strict-encrypt #7460
🎉 Created destination-oracle-strict-encrypt #7460
Conversation
/test connector=connectors/destination-oracle
|
/test connector=connectors/destination-oracle-strict-encrypt
|
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.
LGTM
/test connector=connectors/destination-oracle
|
/test connector=connectors/destination-oracle-strict-encrypt
|
"description": "This parameter defines the encryption algorithm to be used", | ||
"title": "Encryption Algorithm", | ||
"default": "AES256", | ||
"enum": ["AES256", "RC4_56", "3DES168"] |
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.
like described in the UX handbook this seems like an implementation detail. Can we just choose an algorithm so the user doesn't need to?
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.
I will remove this from the specification, and the default value will be added in the code
} | ||
}, | ||
{ | ||
"title": "TLS Encrypted (verify certificate)", |
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't use cert verification generically because it clashes with SSH tunneling like described in the "Interaction between SSH & TLS" section of the Database security RFC -- I think we should remove this for the time being
( we can remove it from the spec, no need to remove it from the code since we may want to implement it in the future)
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.
great, I'll just take that out of the spec.
@sherifnada, I have corrected the inaccuracies indicated by you. Please, look at this pull request again. |
...grations/destination/oracle_strict_encrypt/OracleStrictEncryptDestinationAcceptanceTest.java
Outdated
Show resolved
Hide resolved
...grations/destination/oracle_strict_encrypt/OracleStrictEncryptDestinationAcceptanceTest.java
Outdated
Show resolved
Hide resolved
...grations/destination/oracle_strict_encrypt/OracleStrictEncryptDestinationAcceptanceTest.java
Outdated
Show resolved
Hide resolved
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.
minor comments added
@alexandr-shegeda, I deleted all the comments you mentioned |
/publish connector=connectors/destination-oracle
|
/publish connector=connectors/destination-oracle-strict-encrypt
|
* created destination-oracle-strict-encrypt * fixed code style * updated specNormalizationValueShouldBeCorrect test * fixed remarks * fixed remarks * fixed remarks
What
Created destination-oracle-strict-encrypt
Pre-merge Checklist
Expand the relevant checklist and delete the others.
New Connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/SUMMARY.md
docs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampledocs/integrations/README.md
airbyte-integrations/builds.md
Airbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing./publish
command described hereUpdating a connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing./publish
command described hereConnector Generator
-scaffold
in their name) have been updated with the latest scaffold by running./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates
then checking in your changes