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

Bigquery-denormalized fix integration tests #20640

Conversation

suhomud
Copy link
Contributor

@suhomud suhomud commented Dec 19, 2022

What

in order to fix integration test, spec.json and destination_definitions.yaml should be consistent.
old spec.json:

  "supportsNormalization": false,
  "supportsDBT": true,  

old destination_definitions.yaml:

- name: BigQuery (denormalized typed struct)
  destinationDefinitionId: 079d5540-f236-4294-ba7c-ade8fd918496
  dockerRepository: airbyte/destination-bigquery-denormalized
  dockerImageTag: 1.2.9
  documentationUrl: https://docs.airbyte.com/integrations/destinations/bigquery
  icon: bigquery.svg
  normalizationConfig:
    normalizationRepository: airbyte/normalization
    normalizationTag: 0.2.25
    normalizationIntegrationType: bigquery
  resourceRequirements:
    jobSpecific:
      - jobType: sync
        resourceRequirements:
          memory_limit: "1Gi"
          memory_request: "1Gi"
  releaseStage: beta

result:

BigQueryDenormalizedGcsDestinationAcceptanceTest > specNormalizationValueShouldBeCorrect() FAILED
specNormalizationValueShouldBeCorrect 

new spec.json:

  "supportsNormalization": false,
  "supportsDBT": false,

destination_definitions.yaml:

- name: BigQuery (denormalized typed struct)
  destinationDefinitionId: 079d5540-f236-4294-ba7c-ade8fd918496
  dockerRepository: airbyte/destination-bigquery-denormalized
  dockerImageTag: 1.2.9
  documentationUrl: https://docs.airbyte.com/integrations/destinations/bigquery
  icon: bigquery.svg
  resourceRequirements:
    jobSpecific:
      - jobType: sync
        resourceRequirements:
          memory_limit: "1Gi"
          memory_request: "1Gi"
  releaseStage: beta

How

Describe the solution

Recommended reading order

  1. spec.json
  2. destination_definitions.yaml
  3. BigQueryDenormalizedDestinationAcceptanceTest.java

@suhomud
Copy link
Contributor Author

suhomud commented Dec 19, 2022

/test connector=connectors/destination-bigquery-denormalized

🕑 connectors/destination-bigquery-denormalized https://github.com/airbytehq/airbyte/actions/runs/3732388658
✅ connectors/destination-bigquery-denormalized https://github.com/airbytehq/airbyte/actions/runs/3732388658
Python tests coverage:

Name                                                              Stmts   Miss  Cover
-------------------------------------------------------------------------------------
normalization/transform_config/__init__.py                            2      0   100%
normalization/transform_catalog/reserved_keywords.py                 14      0   100%
normalization/transform_catalog/__init__.py                           2      0   100%
normalization/destination_type.py                                    14      0   100%
normalization/__init__.py                                             4      0   100%
normalization/transform_catalog/destination_name_transformer.py     166      8    95%
normalization/transform_catalog/table_name_registry.py              174     34    80%
normalization/transform_config/transform.py                         189     48    75%
normalization/transform_catalog/utils.py                             51     14    73%
normalization/transform_catalog/dbt_macro.py                         22      7    68%
normalization/transform_catalog/catalog_processor.py                147     80    46%
normalization/transform_catalog/transform.py                         61     38    38%
normalization/transform_catalog/stream_processor.py                 595    400    33%
-------------------------------------------------------------------------------------
TOTAL                                                              1441    629    56%

Build Passed

Test summary info:

All Passed

@github-actions
Copy link
Contributor

github-actions bot commented Dec 19, 2022

Affected Connector Report

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to do the following as needed:

  • Run integration tests
  • Bump connector or module version
  • Add changelog
  • Publish the new version

✅ Sources (0)

Connector Version Changelog Publish
  • See "Actionable Items" below for how to resolve warnings and errors.

✅ Destinations (1)

Connector Version Changelog Publish
destination-bigquery-denormalized 1.2.9
  • See "Actionable Items" below for how to resolve warnings and errors.

✅ Other Modules (0)

Actionable Items

(click to expand)

Category Status Actionable Item
Version
mismatch
The version of the connector is different from its normal variant. Please bump the version of the connector.

doc not found
The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like source-jdbc is not published or documented). Please double-check to make sure that it is not a bug.
Changelog
doc not found
The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like source-jdbc is not published or documented). Please double-check to make sure that it is not a bug.

changelog missing
There is no chnagelog for the current version of the connector. If you are the author of the current version, please add a changelog.
Publish
not in seed
The connector is not in the seed file (e.g. source_definitions.yaml), so its publication status cannot be checked. This can be normal (e.g. some connectors are cloud-specific, and only listed in the cloud seed file). Please double-check to make sure that it is not a bug.

diff seed version
The connector exists in the seed file, but the latest version is not listed there. This usually means that the latest version is not published. Please use the /publish command to publish the latest version.

@suhomud
Copy link
Contributor Author

suhomud commented Dec 19, 2022

/test connector=connectors/destination-bigquery-denormalized

🕑 connectors/destination-bigquery-denormalized https://github.com/airbytehq/airbyte/actions/runs/3732484493
✅ connectors/destination-bigquery-denormalized https://github.com/airbytehq/airbyte/actions/runs/3732484493
Python tests coverage:

Name                                                              Stmts   Miss  Cover
-------------------------------------------------------------------------------------
normalization/transform_config/__init__.py                            2      0   100%
normalization/transform_catalog/reserved_keywords.py                 14      0   100%
normalization/transform_catalog/__init__.py                           2      0   100%
normalization/destination_type.py                                    14      0   100%
normalization/__init__.py                                             4      0   100%
normalization/transform_catalog/destination_name_transformer.py     166      8    95%
normalization/transform_catalog/table_name_registry.py              174     34    80%
normalization/transform_config/transform.py                         189     48    75%
normalization/transform_catalog/utils.py                             51     14    73%
normalization/transform_catalog/dbt_macro.py                         22      7    68%
normalization/transform_catalog/catalog_processor.py                147     80    46%
normalization/transform_catalog/transform.py                         61     38    38%
normalization/transform_catalog/stream_processor.py                 595    400    33%
-------------------------------------------------------------------------------------
TOTAL                                                              1441    629    56%

Build Passed

Test summary info:

All Passed

Copy link
Contributor

@edgao edgao left a comment

Choose a reason for hiding this comment

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

I think this is correct? but @pedroslopez can you confirm

also, isn't destination_definitions generated using the spec.json files? Shouldn't this have caused build failures, since ./gradlew build would have generated a nonempty diff?

Comment on lines -63 to -66
normalizationConfig:
normalizationRepository: airbyte/normalization
normalizationTag: 0.2.25
normalizationIntegrationType: bigquery
Copy link
Contributor

Choose a reason for hiding this comment

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

To confirm: are we saying here that this destination does not support normalization? (from the name -denormalized i would assume so, but confirming 😄 )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it was marked in spec.json

@pedroslopez
Copy link
Contributor

also, isn't destination_definitions generated using the spec.json files? Shouldn't this have caused build failures, since ./gradlew build would have generated a nonempty diff?

@edgao No, definitions are not generated. You may be thinking of destination_specs.yaml which are generated by running a spec command on the connector and saving the results.

There's an upcoming ticket to remove supportsDbt and related fields from the spec and keep that information as part of the definitions only. I think this is it but there's not much info in it 😅 #18239

@suhomud suhomud temporarily deployed to more-secrets December 19, 2022 18:11 — with GitHub Actions Inactive
@suhomud suhomud temporarily deployed to more-secrets December 19, 2022 18:12 — with GitHub Actions Inactive
@grishick grishick self-requested a review December 20, 2022 17:10
@grishick
Copy link
Contributor

/approve-and-merge reason="Need to merge test fixes to unblock others. These changes affect only tests."

@octavia-approvington
Copy link
Contributor

Approve this
imagine the commander saying yes

@octavia-approvington octavia-approvington merged commit eb176f4 into master Dec 27, 2022
@octavia-approvington octavia-approvington deleted the suhomud/fix_bigquery_denormalized_spec_normalization_test branch December 27, 2022 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants