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.

Sign up for GitHub

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

Jump to bottom

Turn on MYSQL normalization flag. #4651

Merged
merged 11 commits into from
Jul 9, 2021
Merged

Turn on MYSQL normalization flag. #4651

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
863a311
Turn on normalization flag. Bump version.
davinchia Jul 9, 2021
2c33294
Update PR build.
davinchia Jul 9, 2021
f176b6b
Supports normalisation is true.
davinchia Jul 9, 2021
485b919
Use normalisation image to make sure the right binaries are present.
davinchia Jul 9, 2021
ae607d7
Checkpoint: Get this to a working state.
davinchia Jul 9, 2021
c5ed287
Fix tests and bump images
ChristopheDuong Jul 9, 2021
debcea2
Update comments
ChristopheDuong Jul 9, 2021
b117b85
Update type_conversions.sql
ChristopheDuong Jul 9, 2021
a9e207c
format code
ChristopheDuong Jul 9, 2021
672b1b2
Add test to remind turning normalization boolean on in spec
ChristopheDuong Jul 9, 2021
c055075
Apply suggestions from code review
ChristopheDuong Jul 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...esources/config/STANDARD_DESTINATION_DEFINITION/ca81ee7c-3163-4246-af40-094cc31e5e42.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
"destinationDefinitionId": "ca81ee7c-3163-4246-af40-094cc31e5e42",
"name": "MySQL",
"dockerRepository": "airbyte/destination-mysql",
"dockerImageTag": "0.1.6",
"dockerImageTag": "0.1.7",
"documentationUrl": "https://docs.airbyte.io/integrations/destinations/mysql"
}
2 changes: 1 addition & 1 deletion airbyte-config/init/src/main/resources/seed/destination_definitions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
- destinationDefinitionId: ca81ee7c-3163-4246-af40-094cc31e5e42
name: MySQL
dockerRepository: airbyte/destination-mysql
dockerImageTag: 0.1.6
dockerImageTag: 0.1.7
documentationUrl: https://docs.airbyte.io/integrations/destinations/mysql
- destinationDefinitionId: d4353156-9217-4cad-8dd7-c108fd4f74cf
name: MS SQL Server
Expand Down
2 changes: 1 addition & 1 deletion ...main/java/io/airbyte/integrations/standardtest/destination/DestinationAcceptanceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,7 @@ void testCustomDbtTransformations() throws Exception {
final OperatorDbt dbtConfig = new OperatorDbt()
.withGitRepoUrl("https://github.com/fishtown-analytics/jaffle_shop.git")
.withGitRepoBranch("main")
.withDockerImage("fishtownanalytics/dbt:0.19.1");
.withDockerImage("airbyte/normalization:dev");
Copy link
Contributor

Choose a reason for hiding this comment

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

just making sure i follow here...so we are using airbyte normalization because it has the right dbt deps. But when we hit dbt run below what are we actually running then? Airbyte normalization?

Copy link
Contributor

@ChristopheDuong ChristopheDuong Jul 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

No, it's just using the dbt binary and packages from the docker image, the models are coming from what is defined in GitRepoUrl (ie https://github.com/fishtown-analytics/jaffle_shop.git)

p.s. it turns out the SQL files in the jaffle shop are not compatible with MySQL ... that's why we had to disable the test until we merge a PR into the jaffle_shop repo... or we could make a fork instead...

//
// jaffle_shop is a fictional ecommerce store maintained by fishtownanalytics/dbt.
//
Expand Down
2 changes: 1 addition & 1 deletion airbyte-integrations/connectors/destination-mysql/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ COPY build/distributions/${APPLICATION}*.tar ${APPLICATION}.tar

RUN tar xf ${APPLICATION}.tar --strip-components=1

LABEL io.airbyte.version=0.1.6
LABEL io.airbyte.version=0.1.7
LABEL io.airbyte.name=airbyte/destination-mysql
2 changes: 1 addition & 1 deletion airbyte-integrations/connectors/destination-mysql/src/main/resources/spec.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"documentationUrl": "https://docs.airbyte.io/integrations/destinations/mysql",
"supportsIncremental": true,
"supportsNormalization": false,
"supportsNormalization": true,
Copy link
Contributor

Choose a reason for hiding this comment

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

why was it turned off?

Copy link
Contributor

@ChristopheDuong ChristopheDuong Jul 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

if has never been ON

Therefore it was OFF because the implementation of normalization for MySQL was not implemented yet when the destination was first implemented

Copy link
Contributor

Choose a reason for hiding this comment

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

but why didn't the PR for normalization turn it on? just missed?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, it missed it

Copy link
Contributor

@ChristopheDuong ChristopheDuong Jul 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

added a test here: https://github.com/airbytehq/airbyte/pull/4651/files#r667002992

"supportsDBT": true,
"supported_destination_sync_modes": ["overwrite", "append"],
"connectionSpecification": {
Expand Down
5 changes: 5 additions & 0 deletions ...ration/java/io/airbyte/integrations/destination/mysql/MySQLDestinationAcceptanceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ protected boolean implementsNamespaces() {
return true;
}

@Override
protected boolean supportsNormalization() {
return true;
}

@Override
protected JsonNode getConfig() {
return Jsons.jsonNode(ImmutableMap.builder()
Expand Down
1 change: 1 addition & 0 deletions docs/integrations/destinations/mysql.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ You should now have all the requirements needed to configure MySQL as a destinat

| Version | Date | Pull Request | Subject |
| :--- | :--- | :--- | :--- |
| 0.1.7 | 2021-07-09 | [#4651](https://github.com/airbytehq/airbyte/pull/4651) | Switch normalization flag on so users can use normalization. |
| 0.1.6 | 2021-07-03 | [#4531](https://github.com/airbytehq/airbyte/pull/4531) | Added normalization for MySQL. |
| 0.1.5 | 2021-07-03 | [#3973](https://github.com/airbytehq/airbyte/pull/3973) | Added `AIRBYTE_ENTRYPOINT` for kubernetes support. |
| 0.1.4 | 2021-07-03 | [#3290](https://github.com/airbytehq/airbyte/pull/3290) | Switched to get states from destination instead of source. |
Expand Down