Skip to content
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

New Destination: Teradata Vantage #20428

Closed
wants to merge 5 commits into from
Closed

New Destination: Teradata Vantage #20428

wants to merge 5 commits into from

Conversation

sc250072
Copy link
Contributor

What

Describe what the change is solving
Teradata Vantage is the complete cloud analytics and data platform with next-generation, cloud-native deployment and expanded analytics capabilities. This connector helps to load data to teradata from other sources and apply transformations through airbyte.
It helps to add screenshots if it affects the frontend.
doesn't effect the frontend

How

The current version of the connector provides importing data to teradata through airbyte. The implemented version of the connector will allow for overwrite and append sync modes. Tried to implement basic and dbt normalization but facing issues on normalization process. Currently, this connector able to load data to teradata into raw tables.

Recommended reading order

  1. TeradataDestination.java
  2. TeradataSqlOperations.java and rest

🚨 User Impact 🚨

No breaking changes. User can load data from any airbyte supported source to teradata through airbyte.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

  • Community member? Grant edit access to maintainers (instructions)
  • Secrets in the connector's spec are annotated with airbyte_secret
  • Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • Code reviews completed
  • Documentation updated
    • Connector's README.md
    • Connector's bootstrap.md. See description and examples
    • docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
    • docs/integrations/README.md
    • airbyte-integrations/builds.md
  • PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

  • Create a non-forked branch based on this PR and test the below items on it
  • Build is successful
  • If new credentials are required for use in CI, add them to GSM. Instructions.
  • /test connector=connectors/<name> command is passing
  • New Connector version released on Dockerhub by running the /publish command described here
  • After the connector is published, connector added to connector index as described here
  • Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here
Updating a connector

Community member or Airbyter

  • Grant edit access to maintainers (instructions)
  • Secrets in the connector's spec are annotated with airbyte_secret
  • Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • Code reviews completed
  • Documentation updated
    • Connector's README.md
    • Connector's bootstrap.md. See description and examples
    • Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
  • PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

  • Create a non-forked branch based on this PR and test the below items on it
  • Build is successful
  • If new credentials are required for use in CI, add them to GSM. Instructions.
  • /test connector=connectors/<name> command is passing
  • New Connector version released on Dockerhub and connector version bumped by running the /publish command described here
Connector Generator
  • Issue acceptance criteria met
  • PR name follows PR naming conventions
  • If adding a new generator, add it to the list of scaffold modules being tested
  • The generator test modules (all connectors with -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
  • Documentation which references the generator is updated as needed

Tests

Unit

image

Integration

Integration/AcceptanceTests requires teradata vantage instance and able to access from machine, where these tests are running. Please follow Prerequisites section of https://github.com/SatishChGit/airbyte/blob/teradata_java_dest_conn/docs/integrations/destinations/teradata.md.
To run Integration/AcceptanceTests, this connector requires config.json and failureconfig.json json files under airbyte-integrations/connectors/destination-teradata/secrets
format of config.json and failureconfig.json is
{ "host" : "teradatainstancehostname", "username": "username", "password": "password", "schema" : "schemaname" }
Below three test cases are failing and need airbyte help to resolve these.
TeradataDestinationAcceptanceTest. [1] exchange_rate_messages.txt, exchange_rate_catalog.json
TeradataDestinationAcceptanceTest. [2] edge_case_messages.txt, edge_case_catalog.json
TeradataDestinationAcceptanceTest. [2] edge_case_messages.txt, edge_case_catalog.json

image

Acceptance

Put your acceptance tests output here.

@sc250072 sc250072 requested a review from a team as a code owner December 13, 2022 12:07
@CLAassistant
Copy link

CLAassistant commented Dec 13, 2022

CLA assistant check
All committers have signed the CLA.

@marcosmarxm
Copy link
Member

marcosmarxm commented Dec 13, 2022

/test connector=connectors/destination-teradata

🕑 connectors/destination-teradata https://github.com/airbytehq/airbyte/actions/runs/3687846710
❌ connectors/destination-teradata https://github.com/airbytehq/airbyte/actions/runs/3687846710
🐛 https://gradle.com/s/3woeon7thve3o

Build Failed

Test summary info:

Could not find result summary

Copy link
Member

@marcosmarxm marcosmarxm left a comment

Choose a reason for hiding this comment

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

I submitted some changes to fix some issues. Your implementation of retrieveRecords was wrong that the reason tests were failing. Now the error looks something with JDBC batchExecute function from Teradata driver.

{"type":"LOG","log":{"level":"ERROR","message":"Something went wrong in the connector. See the logs for more 
details.\nStack Trace: java.lang.RuntimeException: java.sql.BatchUpdateException: [Teradata JDBC Driver] [TeraJDBC
 17.20.00.12] [Error 1338] [SQLState HY000] A failure occurred while executing a PreparedStatement batch request. Details
 of the failure can be found in the exception chain that is accessible with getNextException.\n\tat 
io.airbyte.integrations.destination.teradata.TeradataSqlOperations.lambda$insertRecordsInternal$0(
TeradataSqlOperations.java:64)\n\tat io.airbyte.db.jdbc.DefaultJdbcDatabase.execute(DefaultJdbcDatabase.java:46)
\n\tatio.airbyte.integrations.destination.teradata.TeradataSqlOperations.insertRecordsInternal(TeradataSqlOperations.java
:39)\n\tat io.airbyte.integrations.destination.jdbc.JdbcSqlOperations.insertRecords(JdbcSqlOperations.java:131)\n\tat 
io.airbyte.integrations.destination.jdbc.JdbcBufferedConsumerFactory.lambda$recordWriterFunction$2(JdbcBufferedC
onsumerFactory.java:151)\n\tatio.airbyte.integrations.destination.record_buffer.InMemoryRecordBufferingStrategy.flushAll
(InMemoryRecordBufferingStrategy.java:86)\n\tatio.airbyte.integrations.destination.buffered_stream_consumer.BufferedSt
reamConsumer.close(BufferedStreamConsumer.java:172)\n\taio.airbyte.integrations.base.FailureTrackingAirbyte
MessageConsumer.close(FailureTrackingAirbyteMessageConsumer.java:64)\n\tat io.airbyte.integrations.base.IntegrationRunner.runInternal(
IntegrationRunner.java:149)\n\tat io.airbyte.integrations.base.IntegrationRunner.run(IntegrationRunner.java:98)\n\tat
 io.airbyte.integrations.destination.teradata.TeradataDestination.main(TeradataDestination.java:34)\nCaused by:
 java.sql.BatchUpdateException: [Teradata JDBC Driver] [TeraJDBC 17.20.00.12] [Error 1338] [SQLState HY000] A failure
 occurred while executing a PreparedStatement batch request. Details of the failure can be found in the exception chain that
 is accessible with getNextException.\n\tat
 com.teradata.jdbc.jdbc_4.util.ErrorFactory.makeBatchUpdateException(ErrorFactory.java:149)\n\tat
 com.teradata.jdbc.jdbc_4.util.ErrorFactory.makeBatchUpdateException(ErrorFactory.java:138)\n\tat
 com.teradata.jdbc.jdbc_4.TDPreparedStatement.executeBatchDMLArray(TDPreparedStatement.java:304)\n\tat
 com.teradata.jdbc.jdbc_4.TDPreparedStatement.executeBatch(TDPreparedStatement.java:2862)\n\tat
 com.zaxxer.hikari.pool.ProxyStatement.executeBatch(ProxyStatement.java:127)\n\tat
 com.zaxxer.hikari.pool.HikariProxyPreparedStatement.executeBatch(HikariProxyPreparedStatement.java)\n\tat io.airbyte.integrations.destination.teradata.TeradataSqlOperations.lambda$insertRecordsInternal$0(TeradataSqlOperations.java:58)\n\t... 10 more\nCaused by: java.sql.SQLException: [Teradata JDBC Driver] [TeraJDBC 17.20.00.12] [Error 1339] [SQLState HY000] A failure occurred while executing a PreparedStatement batch request. The parameter set was not executed and should be resubmitted individually using the PreparedStatement executeUpdate method.\n\tat com.teradata.jdbc.jdbc_4.util.ErrorFactory.makeDriverJDBCException(ErrorFactory.java:95)\n\tat com.teradata.jdbc.jdbc_4.util.ErrorFactory.makeDriverJDBCException(ErrorFactory.java:65)\n\tat com.teradata.jdbc.jdbc_4.statemachine.PreparedBatchStatementController.handleRunException(PreparedBatchStatementController.java:96)\n\tat com.teradata.jdbc.jdbc_4.statemachine.StatementController.runBody(StatementController.java:145)\n\tat com.teradata.jdbc.jdbc_4.statemachine.PreparedBatchStatementController.run(PreparedBatchStatementController.java:58)\n\tat com.teradata.jdbc.jdbc_4.TDStatement.executeStatement(TDStatement.java:389)\n\tat com.teradata.jdbc.jdbc_4.TDPreparedStatement.executeBatchDMLArray(TDPreparedStatement.java:284)\n\t... 14 more\n"}}

@marcosmarxm
Copy link
Member

marcosmarxm commented Dec 14, 2022

/test connector=connectors/destination-teradata

🕑 connectors/destination-teradata https://github.com/airbytehq/airbyte/actions/runs/3690419347
❌ connectors/destination-teradata https://github.com/airbytehq/airbyte/actions/runs/3690419347
🐛 https://gradle.com/s/glksmat7x5s7g

Build Failed

Test summary info:

Could not find result summary

@marcosmarxm marcosmarxm self-assigned this Dec 14, 2022
@marcosmarxm
Copy link
Member

marcosmarxm commented Dec 16, 2022

/test connector=connectors/destination-teradata

🕑 connectors/destination-teradata https://github.com/airbytehq/airbyte/actions/runs/3715424679
❌ connectors/destination-teradata https://github.com/airbytehq/airbyte/actions/runs/3715424679
🐛 https://gradle.com/s/fjnxna4igqmiw

Build Failed

Test summary info:

Could not find result summary

@sc250072
Copy link
Contributor Author

I submitted some changes to fix some issues. Your implementation of retrieveRecords was wrong that the reason tests were failing. Now the error looks something with JDBC batchExecute function from Teradata driver.

{"type":"LOG","log":{"level":"ERROR","message":"Something went wrong in the connector. See the logs for more 
details.\nStack Trace: java.lang.RuntimeException: java.sql.BatchUpdateException: [Teradata JDBC Driver] [TeraJDBC
 17.20.00.12] [Error 1338] [SQLState HY000] A failure occurred while executing a PreparedStatement batch request. Details
 of the failure can be found in the exception chain that is accessible with getNextException.\n\tat 
io.airbyte.integrations.destination.teradata.TeradataSqlOperations.lambda$insertRecordsInternal$0(
TeradataSqlOperations.java:64)\n\tat io.airbyte.db.jdbc.DefaultJdbcDatabase.execute(DefaultJdbcDatabase.java:46)
\n\tatio.airbyte.integrations.destination.teradata.TeradataSqlOperations.insertRecordsInternal(TeradataSqlOperations.java
:39)\n\tat io.airbyte.integrations.destination.jdbc.JdbcSqlOperations.insertRecords(JdbcSqlOperations.java:131)\n\tat 
io.airbyte.integrations.destination.jdbc.JdbcBufferedConsumerFactory.lambda$recordWriterFunction$2(JdbcBufferedC
onsumerFactory.java:151)\n\tatio.airbyte.integrations.destination.record_buffer.InMemoryRecordBufferingStrategy.flushAll
(InMemoryRecordBufferingStrategy.java:86)\n\tatio.airbyte.integrations.destination.buffered_stream_consumer.BufferedSt
reamConsumer.close(BufferedStreamConsumer.java:172)\n\taio.airbyte.integrations.base.FailureTrackingAirbyte
MessageConsumer.close(FailureTrackingAirbyteMessageConsumer.java:64)\n\tat io.airbyte.integrations.base.IntegrationRunner.runInternal(
IntegrationRunner.java:149)\n\tat io.airbyte.integrations.base.IntegrationRunner.run(IntegrationRunner.java:98)\n\tat
 io.airbyte.integrations.destination.teradata.TeradataDestination.main(TeradataDestination.java:34)\nCaused by:
 java.sql.BatchUpdateException: [Teradata JDBC Driver] [TeraJDBC 17.20.00.12] [Error 1338] [SQLState HY000] A failure
 occurred while executing a PreparedStatement batch request. Details of the failure can be found in the exception chain that
 is accessible with getNextException.\n\tat
 com.teradata.jdbc.jdbc_4.util.ErrorFactory.makeBatchUpdateException(ErrorFactory.java:149)\n\tat
 com.teradata.jdbc.jdbc_4.util.ErrorFactory.makeBatchUpdateException(ErrorFactory.java:138)\n\tat
 com.teradata.jdbc.jdbc_4.TDPreparedStatement.executeBatchDMLArray(TDPreparedStatement.java:304)\n\tat
 com.teradata.jdbc.jdbc_4.TDPreparedStatement.executeBatch(TDPreparedStatement.java:2862)\n\tat
 com.zaxxer.hikari.pool.ProxyStatement.executeBatch(ProxyStatement.java:127)\n\tat
 com.zaxxer.hikari.pool.HikariProxyPreparedStatement.executeBatch(HikariProxyPreparedStatement.java)\n\tat io.airbyte.integrations.destination.teradata.TeradataSqlOperations.lambda$insertRecordsInternal$0(TeradataSqlOperations.java:58)\n\t... 10 more\nCaused by: java.sql.SQLException: [Teradata JDBC Driver] [TeraJDBC 17.20.00.12] [Error 1339] [SQLState HY000] A failure occurred while executing a PreparedStatement batch request. The parameter set was not executed and should be resubmitted individually using the PreparedStatement executeUpdate method.\n\tat com.teradata.jdbc.jdbc_4.util.ErrorFactory.makeDriverJDBCException(ErrorFactory.java:95)\n\tat com.teradata.jdbc.jdbc_4.util.ErrorFactory.makeDriverJDBCException(ErrorFactory.java:65)\n\tat com.teradata.jdbc.jdbc_4.statemachine.PreparedBatchStatementController.handleRunException(PreparedBatchStatementController.java:96)\n\tat com.teradata.jdbc.jdbc_4.statemachine.StatementController.runBody(StatementController.java:145)\n\tat com.teradata.jdbc.jdbc_4.statemachine.PreparedBatchStatementController.run(PreparedBatchStatementController.java:58)\n\tat com.teradata.jdbc.jdbc_4.TDStatement.executeStatement(TDStatement.java:389)\n\tat com.teradata.jdbc.jdbc_4.TDPreparedStatement.executeBatchDMLArray(TDPreparedStatement.java:284)\n\t... 14 more\n"}}

The issue is with space of database. The database added for test cases execution has less space than required. Increased size of the database space to get test cases success.

@sc250072 sc250072 requested a review from marcosmarxm December 20, 2022 15:47

| Version | Date | Pull Request | Subject |
|:--------|:-----------|:--------------------------------------------------------------|:---------------------------------|
| 0.1.0 | 2022-12-13 | https://github.com/airbytehq/airbyte/pull/20428 | New Destination Teradata Vantage |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@marcosmarxm can we use the 0.0.1 as initial version of teradata vantage connector instead of 0.1.0?

Copy link
Member

Choose a reason for hiding this comment

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

The Airbyte default is 0.1.0

@marcosmarxm
Copy link
Member

marcosmarxm commented Dec 20, 2022

/test connector=connectors/destination-teradata

🕑 connectors/destination-teradata https://github.com/airbytehq/airbyte/actions/runs/3742102865
✅ connectors/destination-teradata https://github.com/airbytehq/airbyte/actions/runs/3742102865
No Python unittests run

Build Passed

Test summary info:

All Passed


| Version | Date | Pull Request | Subject |
|:--------|:-----------|:--------------------------------------------------------------|:---------------------------------|
| 0.1.0 | 2022-12-13 | https://github.com/airbytehq/airbyte/pull/20428 | New Destination Teradata Vantage |
Copy link
Member

Choose a reason for hiding this comment

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

The Airbyte default is 0.1.0

@marcosmarxm
Copy link
Member

Hello 👋:skin-tone-2: and thank you for your contribution!

Airbyte has instituted a code freeze between 19 and 30 December, to make sure there are no disruptions during the holidays.
Because of this, reviewing and merging your contribution may take longer than usual.
We apologize for the delay, but we want everyone to have a quiet and happy holiday.

If you have any questions or need further clarification, please don't hesitate to ping via Slack.

@@ -0,0 +1,5 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

was this file supposed to be included? (looks like .gitignore above is trying to exclude this file). If you meant to include a sample config file please prefix the file name with sample_ and don't use real working credentials in it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this file is not using in tests instead using secrets/config.json. Deleted the file.

@sc250072 sc250072 requested a review from grishick December 27, 2022 13:09
Copy link
Contributor

@grishick grishick left a comment

Choose a reason for hiding this comment

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

Thank you for the new connector! I think we can merge this. However, we cannot add this to Airbyte Cloud until we add enforcement of encryption in transit similar to how we do with all other JDBC databases.
The minimum change we need to add this connector to Airbyte Cloud is for it to enforce SSLMODE=require (per Teradata JDBC driver documentation: https://teradata-docs.s3.amazonaws.com/doc/connectivity/jdbc/reference/current/frameset.html). Ideally, it will also support other SSL modes, but that's not strictly required.

@sc250072 sc250072 requested a review from a team January 7, 2023 03:15
@grishick
Copy link
Contributor

grishick commented Jan 9, 2023

/test connector=connectors/destination-teradata

🕑 connectors/destination-teradata https://github.com/airbytehq/airbyte/actions/runs/3870596330

@grishick
Copy link
Contributor

grishick commented Jan 9, 2023

/test connector=connectors/destination-teradata

🕑 connectors/destination-teradata https://github.com/airbytehq/airbyte/actions/runs/3875736040

@grishick
Copy link
Contributor

grishick commented Jan 9, 2023

@SatishChGit looks like some integration tests started failing. It would be easier to see what's wrong if the tests printed out the mismatching data that they are validating. E.g.: instead of

assertTrue(status.getMessage().contains("SQLState 28000"));

have something like:

assertTrue(status.getMessage().contains("SQLState 28000"), status.getMessage());

so that we can see what is wrong in the test output

@grishick
Copy link
Contributor

grishick commented Jan 9, 2023

/test connector=connectors/destination-teradata

🕑 connectors/destination-teradata https://github.com/airbytehq/airbyte/actions/runs/3875910117
❌ connectors/destination-teradata https://github.com/airbytehq/airbyte/actions/runs/3875910117
🐛

@grishick
Copy link
Contributor

grishick commented Jan 9, 2023

/test connector=connectors/destination-teradata

@grishick
Copy link
Contributor

This connector was merged here: #21160
🎉

@grishick grishick closed this Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

6 participants