🎉Destination-bigquery-denorm: fixed emittedAt timestamp #11978

etsybaev · 2022-04-13T17:13:23Z

What

Currently, datetime in the emittedAt field is incorrect

How

Fixed date and checked locally, see captures below

Describe the solution

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

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

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

mkhokh-33 · 2022-04-14T12:27:39Z

.../airbyte/integrations/destination/bigquery/util/TestBigQueryDenormalizedRecordFormatter.java

+  @Override
+  public void addAirbyteColumns(ObjectNode data,
+                                AirbyteRecordMessage recordMessage) {
+    super.addAirbyteColumns(data, recordMessage);


why do we created this class if overwritten method only call super implementation?

This is a test class is located in the test package and exposes a protected method from the main package.
@VisibleForTesting annotation doesn't work here, I didn't want to use reflection to keep code more readable.
Added a comment to code to make it more clear for those who will see this class in the future.

mkhokh-33 · 2022-04-14T12:28:31Z

...rc/test/java/io/airbyte/integrations/destination/bigquery/BigQueryDenormalizedUtilsTest.java

+  @Test
+  public void testEmittedAtTimeConversion() {
+    // in this test we don't care about json schema and naming resolver, so leave it as a null
+    final JsonNode jsonNodeSchema = getSchema();


could we consider to use mock instead of instance?

Just forgot to remove the comment, it's not applicable for now.
Record formatted requires a valid schema for now.
But you were right, even in new code mock is also possible.
Updated, Thanks

etsybaev · 2022-04-14T20:21:00Z

/test connector=connectors/destination-bigquery-denormalized

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

Name                                                                                                                            Stmts   Miss  Cover
---------------------------------------------------------------------------------------------------------------------------------------------------
normalization/transform_config/__init__.py                                                                                          2      0   100%
normalization/transform_catalog/reserved_keywords.py                                                                               13      0   100%
normalization/transform_catalog/__init__.py                                                                                         2      0   100%
normalization/destination_type.py                                                                                                  13      0   100%
normalization/__init__.py                                                                                                           4      0   100%
/actions-runner/_work/airbyte/airbyte/airbyte-integrations/bases/airbyte-protocol/airbyte_protocol/models/airbyte_protocol.py     124      0   100%
/actions-runner/_work/airbyte/airbyte/airbyte-integrations/bases/airbyte-protocol/airbyte_protocol/models/__init__.py               1      0   100%
/actions-runner/_work/airbyte/airbyte/airbyte-integrations/bases/airbyte-protocol/airbyte_protocol/__init__.py                      2      0   100%
normalization/transform_catalog/destination_name_transformer.py                                                                   155      8    95%
normalization/transform_config/transform.py                                                                                       159     31    81%
normalization/transform_catalog/table_name_registry.py                                                                            174     34    80%
normalization/transform_catalog/utils.py                                                                                           34      7    79%
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                                                                               534    345    35%
---------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                                                                                            1447    550    62%

alexandr-shegeda

LGTM. we can proceed with merging and publishing without airbyte review

…-fixed-timestamp

etsybaev · 2022-04-15T12:56:39Z

/test connector=connectors/destination-bigquery-denormalized

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

Name                                                                                                                            Stmts   Miss  Cover
---------------------------------------------------------------------------------------------------------------------------------------------------
normalization/transform_config/__init__.py                                                                                          2      0   100%
normalization/transform_catalog/reserved_keywords.py                                                                               13      0   100%
normalization/transform_catalog/__init__.py                                                                                         2      0   100%
normalization/destination_type.py                                                                                                  13      0   100%
normalization/__init__.py                                                                                                           4      0   100%
/actions-runner/_work/airbyte/airbyte/airbyte-integrations/bases/airbyte-protocol/airbyte_protocol/models/airbyte_protocol.py     124      0   100%
/actions-runner/_work/airbyte/airbyte/airbyte-integrations/bases/airbyte-protocol/airbyte_protocol/models/__init__.py               1      0   100%
/actions-runner/_work/airbyte/airbyte/airbyte-integrations/bases/airbyte-protocol/airbyte_protocol/__init__.py                      2      0   100%
normalization/transform_catalog/destination_name_transformer.py                                                                   155      8    95%
normalization/transform_config/transform.py                                                                                       168     31    82%
normalization/transform_catalog/table_name_registry.py                                                                            174     34    80%
normalization/transform_catalog/utils.py                                                                                           33      7    79%
normalization/transform_catalog/catalog_processor.py                                                                              143     77    46%
normalization/transform_catalog/transform.py                                                                                       45     26    42%
normalization/transform_catalog/stream_processor.py                                                                               524    337    36%
---------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                                                                                            1403    520    63%

etsybaev · 2022-04-15T13:55:41Z

/publish connector=connectors/destination-bigquery-denormalized

🕑 connectors/destination-bigquery-denormalized https://github.com/airbytehq/airbyte/actions/runs/2172836927
🚀 Successfully published connectors/destination-bigquery-denormalized
🚀 Auto-bumped version for connectors/destination-bigquery-denormalized
✅ connectors/destination-bigquery-denormalized https://github.com/airbytehq/airbyte/actions/runs/2172836927

* [11067] Destination-bigquery-denorm: fixed emittedAt timestamp

[11067] Destination-bigquery-denorm: fixed emittedAt timestamp

30a144d

github-actions bot added the area/connectors Connector related issues label Apr 13, 2022

etsybaev linked an issue Apr 13, 2022 that may be closed by this pull request

Wrong Timestamp while using BigQuery(Denormalized) Destination #11067

Closed

etsybaev requested review from alexandr-shegeda and mkhokh-33 April 14, 2022 10:18

mkhokh-33 suggested changes Apr 14, 2022

View reviewed changes

etsybaev added 3 commits April 14, 2022 22:29

reverted

d6866af

removed obsolete comment

92e6f60

mocked formatter

cb99c4c

etsybaev force-pushed the etsybaev/11067-destination-bigquery-denorm-fixed-timestamp branch from c12cb0b to cb99c4c Compare April 14, 2022 20:11

etsybaev changed the title ~~[11067] Destination-bigquery-denorm: fixed emittedAt timestamp~~ 🎉Destination-bigquery-denorm: fixed emittedAt timestamp Apr 14, 2022

etsybaev requested a review from mkhokh-33 April 15, 2022 07:38

mkhokh-33 approved these changes Apr 15, 2022

View reviewed changes

alexandr-shegeda approved these changes Apr 15, 2022

View reviewed changes

etsybaev marked this pull request as ready for review April 15, 2022 12:40

Merge branch 'master' into etsybaev/11067-destination-bigquery-denorm…

99b1384

…-fixed-timestamp

bumped version

6ff4ee8

github-actions bot added the area/documentation Improvements or additions to documentation label Apr 15, 2022

auto-bump connector version

eae8bc9

octavia-squidington-iii temporarily deployed to more-secrets April 15, 2022 14:33 Inactive

etsybaev merged commit 281e0ee into master Apr 15, 2022

etsybaev deleted the etsybaev/11067-destination-bigquery-denorm-fixed-timestamp branch April 15, 2022 14:39

octavia-squidington-iii mentioned this pull request Apr 19, 2022

Bump Airbyte version from 0.36.0-alpha to 0.36.1-alpha #12148

Merged

suhomud pushed a commit that referenced this pull request May 23, 2022

🎉Destination-bigquery-denorm: fixed emittedAt timestamp (#11978)

c6df2ea

* [11067] Destination-bigquery-denorm: fixed emittedAt timestamp

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🎉Destination-bigquery-denorm: fixed emittedAt timestamp #11978

🎉Destination-bigquery-denorm: fixed emittedAt timestamp #11978

etsybaev commented Apr 13, 2022 •

edited

Loading

mkhokh-33 Apr 14, 2022

etsybaev Apr 14, 2022 •

edited

Loading

mkhokh-33 Apr 14, 2022

etsybaev Apr 14, 2022 •

edited

Loading

etsybaev commented Apr 14, 2022 •

edited by github-actions bot

Loading

alexandr-shegeda left a comment •

edited

Loading

etsybaev commented Apr 15, 2022 •

edited by github-actions bot

Loading

etsybaev commented Apr 15, 2022 •

edited by github-actions bot

Loading

🎉Destination-bigquery-denorm: fixed emittedAt timestamp #11978

🎉Destination-bigquery-denorm: fixed emittedAt timestamp #11978

Conversation

etsybaev commented Apr 13, 2022 • edited Loading

What

How

Recommended reading order

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

mkhokh-33 Apr 14, 2022

Choose a reason for hiding this comment

etsybaev Apr 14, 2022 • edited Loading

Choose a reason for hiding this comment

mkhokh-33 Apr 14, 2022

Choose a reason for hiding this comment

etsybaev Apr 14, 2022 • edited Loading

Choose a reason for hiding this comment

etsybaev commented Apr 14, 2022 • edited by github-actions bot Loading

alexandr-shegeda left a comment • edited Loading

Choose a reason for hiding this comment

etsybaev commented Apr 15, 2022 • edited by github-actions bot Loading

etsybaev commented Apr 15, 2022 • edited by github-actions bot Loading

etsybaev commented Apr 13, 2022 •

edited

Loading

etsybaev Apr 14, 2022 •

edited

Loading

etsybaev Apr 14, 2022 •

edited

Loading

etsybaev commented Apr 14, 2022 •

edited by github-actions bot

Loading

alexandr-shegeda left a comment •

edited

Loading

etsybaev commented Apr 15, 2022 •

edited by github-actions bot

Loading

etsybaev commented Apr 15, 2022 •

edited by github-actions bot

Loading