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

🚨🚨✨ Source Instagram: Add primary keys for UserLifetimeInsights and UserInsights; add airbyte_type to timestamp fields #32500

Merged
merged 10 commits into from
Nov 17, 2023
Merged

🚨🚨✨ Source Instagram: Add primary keys for UserLifetimeInsights and UserInsights; add airbyte_type to timestamp fields #32500

merged 10 commits into from
Nov 17, 2023

Conversation

tolik0
Copy link
Contributor

@tolik0 tolik0 commented Nov 14, 2023 •
edited
Loading

What

This PR adds a default primary key to UserLifetimeInsights and UserInsights streams and updates timestamp fields to include timezone information, enhancing data integrity and consistency.

How

  • Primary Keys Update: The primary keys for UserLifetimeInsights have been set to ["business_account_id", "metric", "date"], and for UserInsights to ["business_account_id", "date"]. This change ensures that records are uniquely identified.
  • Timestamp Field Format: The format of timestamp fields has been updated to include "airbyte_type": "timestamp_with_timezone". Additionally, timestamps are now transformed to the correct RFC3339 format. This modification aligns the timestamp fields with Airbyte's data standards.

🚨 User Impact 🚨

This PR introduces breaking changes due to adding the primary key for UserLifetimeInsights and UserInsights streams and changing field type by adding airbyte_type for timestamp fields. Users will have to refresh their schemas and refresh data after the upgrade.

@vercel Vercel
Copy link

vercel bot commented Nov 14, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
airbyte-docs ⬜️ Ignored (Inspect) Visit Preview Nov 17, 2023 3:22pm

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 14, 2023 •
edited by tolik0
Loading

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

  • PR name follows PR naming conventions
  • Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan.
  • Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
  • You've updated the connector's metadata.yaml file any other relevant changes, including a breakingChanges entry for major version bumps. See metadata.yaml docs
  • Secrets in the connector's spec are annotated with airbyte_secret
  • All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
  • Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
  • Migration guide updated in docs/integrations/<source or destination>/<name>-migrations.md with an entry for the new version, if the version is a breaking change. See migration guide example
  • If set, you've ensured the icon is present in the platform-internal repo. (Docs)

If the checklist is complete, but the CI check is failing,

  1. Check for hidden checklists in your PR description

  2. Toggle the github label checklist-action-run on/off to re-run the checklist CI.

@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Nov 14, 2023
@tolik0 tolik0 marked this pull request as ready for review November 14, 2023 14:38
@octavia-squidington-iv octavia-squidington-iv requested review from a team November 14, 2023 14:39
@tolik0 tolik0 requested a review from artem1205 November 15, 2023 10:22
artem1205
artem1205 reviewed Nov 15, 2023
View reviewed changes
airbyte-integrations/connectors/source-instagram/source_instagram/streams.py Outdated


class InstagramStream(Stream, ABC):
"""Base stream class"""

page_size = 100
primary_key = "id"
# Define fields to fix as tuples representing the path to the field
fix_timestamp_fields = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe we could remove this attribute and use schema as a source of truth in order to find date-time fields. (take a look at the facebook pages implementation)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@tolik0 tolik0 self-assigned this Nov 16, 2023
katmarkham
katmarkham requested changes Nov 16, 2023
View reviewed changes
docs/integrations/sources/instagram-migrations.md Outdated

## Upgrading to 2.0.0

This release adds a default primary key for the streams UserLifetimeInsights and UserInsights, and updates the format of timestamp fields to include timezone information.
Copy link
Contributor

Choose a reason for hiding this comment

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

This release adds a default primary key for the streams UserLifetimeInsights and UserInsights, and updates the format of timestamp fields in the Media and Stories streams to include timezone information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

airbyte-integrations/connectors/source-instagram/metadata.yaml Outdated
2.0.0:
message:
This release introduces a default primary key for the streams UserLifetimeInsights and UserInsights.
Additionally, the format of timestamp fields has been updated to include timezone information.
Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally, the format of timestamp fields has been updated in the Media and Stories streams to include timezone information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@tolik0 tolik0 force-pushed the tolik0/source-instagram/add-missing-primary-key branch from 87b553e to 73417e1 Compare November 16, 2023 23:34
maxi297
maxi297 reviewed Nov 17, 2023
View reviewed changes
airbyte-integrations/connectors/source-instagram/unit_tests/test_common.py Outdated
@@ -2,6 +2,7 @@
# Copyright (c) 2023 Airbyte, Inc., all rights reserved.
#

import pytest
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any usage for that import. What is it there for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@tolik0 tolik0 force-pushed the tolik0/source-instagram/add-missing-primary-key branch from dcb0c8d to 2759d1d Compare November 17, 2023 14:34
@octavia-squidington-iv octavia-squidington-iv requested a review from a team November 17, 2023 14:35
@tolik0 tolik0 requested a review from maxi297 November 17, 2023 14:37
@tolik0 tolik0 merged commit cc7f019 into master Nov 17, 2023
26 checks passed
@tolik0 tolik0 deleted the tolik0/source-instagram/add-missing-primary-key branch November 17, 2023 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@katmarkham katmarkham katmarkham approved these changes

@artem1205 artem1205 artem1205 approved these changes

@bazarnov bazarnov bazarnov approved these changes

@maxi297 maxi297 Awaiting requested review from maxi297

Assignees

@tolik0 tolik0

Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/instagram team/connectors-python
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tolik0 @maxi297 @bazarnov @artem1205 @katmarkham @octavia-squidington-iii