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

🐛 Source S3: Up CDK to fix schema type issue #55694

Merged
merged 4 commits into from
Mar 13, 2025

Conversation

maxi297
Copy link
Contributor

@maxi297 maxi297 commented Mar 11, 2025

What

Applying fix where discovered schema is failing when column name is type. For more information, see airbytehq/airbyte-python-cdk#417.

How

Updating the CDK version

Review guide

User Impact

If the csv has type as a header name, the csv parsing should now work.

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

Copy link

vercel bot commented Mar 11, 2025

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

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 13, 2025 1:38am

@@ -33,6 +33,20 @@


class SourceS3StreamReader(AbstractFileBasedStreamReader):
def get_file_acl_permissions(self, file: RemoteFile, logger: logging.Logger) -> Dict[str, Any]:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given the CDK version update fixes the issue, I'm not sure what to do with those newly added methods

Copy link
Contributor

Choose a reason for hiding this comment

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

Soon a fix for this here.

Copy link
Contributor

Choose a reason for hiding this comment

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

@maxi297 can you update your CDK Dev branch and create a new pre-dev version?

This should be fine after v6.38.5

Fine = You can remove noop methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll test that tomorrow, thanks Aldo!

@maxi297 maxi297 marked this pull request as ready for review March 13, 2025 01:23
Copy link
Contributor

@aldogonzalez8 aldogonzalez8 left a comment

Choose a reason for hiding this comment

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

Approved

@maxi297
Copy link
Contributor Author

maxi297 commented Mar 13, 2025

/approve-regression-tests "Was tested with a dev release by Tyler"

Check job output.

✅ Approving regression tests

@maxi297 maxi297 merged commit 8aaaad4 into master Mar 13, 2025
27 checks passed
@maxi297 maxi297 deleted the maxi297/attempted-fix-for-filebase-schema branch March 13, 2025 12:34
sc250072 added a commit to Teradata/airbyte that referenced this pull request Mar 14, 2025
* ✨ Source Intercom: adding a mock server test (airbytehq#54715)

* [Destination MS SQL V2] Correct Part Size, No buffered input stream (airbytehq#55252)

* [source-hibob] Changed check stream from payrolls to profiles airbytehq#55674 (airbytehq#55675)

* Add llms.txt to docs.airbyte.com (airbytehq#55261)

* ✨ TikTok Marketing Source: Add `Pixels`, `PixelInstantPageEvents`, `PixelEventsStatistics` streams (airbytehq#55669)

Co-authored-by: Octavia Squidington III <[email protected]>

* chore(ci): remove stale and unused workflows (airbytehq#55260)

* ci: for community CI, rename 'early ci' to 'pre-release checks', skip duplicated tests in 'connector tests' (airbytehq#55241)

* (feat: Salesloft) - Add emails_scoped_fields stream (airbytehq#55229)

* 🐛 Source Outreach: remove stream_state interpolation (airbytehq#55180)

Co-authored-by: Natik Gadzhi <[email protected]>
Co-authored-by: Octavia Squidington III <[email protected]>

* fix(source-stripe): disable progressive rollout (airbytehq#55682)

Signed-off-by: Artem Inzhyyants <[email protected]>

* fix(source-instagram): Disable cache for InstagramMediaChildrenTransformation (airbytehq#55685)

* [Destination MSSQL V2] Bulk Load Local Performance Test (airbytehq#55687)

* [Destination-S3] File Xfer Local Performance Test (airbytehq#55220)

* 🐛bug(source-hubspot): fix deals_archived and marketing_emails issues for CAT (airbytehq#54177)

* ✨Source Quickbooks: Migrate to manifest-only (airbytehq#55263)

* ✨ Source Zendesk Chat : Migrate to Manifest-only (airbytehq#47319)

* ✨ Source Facebook Marketing: Add `learning_stage_info` field to AdSets stream (airbytehq#50418)

Co-authored-by: Marcos Marx <[email protected]>
Co-authored-by: marcosmarxm <[email protected]>

* Source Sendgrid: Update manifest for adapting changes with AsyncRetriever (airbytehq#55185)

Co-authored-by: Octavia Squidington III <[email protected]>

* docs: fix broken markup in Python CDK Basic Concepts page (airbytehq#55699)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>

* Source Zendesk Talk : Restore Unit Test (airbytehq#50956)

Co-authored-by: Octavia Squidington III <[email protected]>
Co-authored-by: Natik Gadzhi <[email protected]>

* chore: 2.0.0 release (airbytehq#55684)

* Adding EnrichedAirbyteValue and DeclaredField (airbytehq#55218)

Co-authored-by: Octavia Squidington III <[email protected]>

* Add Airbyte Academy section (airbytehq#49964)

* [source-faker] - Bump to stable 6.2.21 (airbytehq#55705)

* fix: remove duplicate breaking changes from destination-mssql metadata (airbytehq#55718)

* fix: restore definition ID (airbytehq#55720)

* ✨ Source Freshdesk : Migrate to Manifest-only (airbytehq#54687)

Co-authored-by: Octavia Squidington III <[email protected]>

* chore(source-s3): update base image to 4.0.0 and use caret dependencies (do not merge) (airbytehq#55202)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
Co-authored-by: Octavia Squidington III <[email protected]>
Co-authored-by: Aldo Gonzalez <[email protected]>

* fix(Source-LinkedIn-Ads): Update outdated schema (airbytehq#55724)

* Bump @babel/runtime-corejs3 from 7.23.6 to 7.26.10 in /docusaurus (airbytehq#55708)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* 🐛 Source S3: Up CDK to fix schema type issue (airbytehq#55694)

* source-shipstation contribution from suhl79 (airbytehq#55738)

Co-authored-by: Octavia Squidington III <[email protected]>
Co-authored-by: Marcos Marx <[email protected]>

* destination-teradata: Upgrade JDBC driver (airbytehq#55183)

Co-authored-by: Marcos Marx <[email protected]>

* 🐛 Destination Databricks: Fix destination check test table collisions when multiple connections write to same schema. (airbytehq#55232)

Co-authored-by: Octavia Squidington III <[email protected]>

* (source-sendgrid) - Configure max concurrent async job count (airbytehq#55744)

* pass streams to debezium sources on cold start (airbytehq#55734)

* Destination S3 Data Lake: exclude invalid fields from identifier fields (airbytehq#55700)

* [source-mysql] pin to cdk 0.342 (airbytehq#55754)

* docs: add enterprise connector documentation (airbytehq#55751)

* Add UTM source (airbytehq#55733)

* Matteogp/docs sap hana update 1 (airbytehq#55696)

* Destination S3 Data Lake: Handle number in primary key (airbytehq#55755)

* Fix reversed assertions in MySQL source tests (airbytehq#55756)

* Update CDK to pass DestinationRecordRaw around (airbytehq#55737)

Co-authored-by: Octavia Squidington III <[email protected]>
Co-authored-by: Edward Gao <[email protected]>

* fix(ci): remove empty notify-on-push workflow file (airbytehq#55757)

* Update OTEL metrics, add new linting exceptions (airbytehq#55752)

* 11526 second pass through iceberg documentation (airbytehq#55736)

* [source-mysql] don't do sampling for source-mysql (airbytehq#55761)

Co-authored-by: Octavia Squidington III <[email protected]>

* 🚨Source Fauna: Migrate to poetry (airbytehq#41051)

---------

Signed-off-by: Artem Inzhyyants <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Maxime Carbonneau-Leclerc <[email protected]>
Co-authored-by: Johnny Schmidt <[email protected]>
Co-authored-by: tautvydas-v <[email protected]>
Co-authored-by: Ian Alton <[email protected]>
Co-authored-by: Tope Folorunso <[email protected]>
Co-authored-by: Octavia Squidington III <[email protected]>
Co-authored-by: Natik Gadzhi <[email protected]>
Co-authored-by: Aaron ("AJ") Steers <[email protected]>
Co-authored-by: Tyler B <[email protected]>
Co-authored-by: kyleromines <[email protected]>
Co-authored-by: Artem Inzhyyants <[email protected]>
Co-authored-by: Anatolii Yatsuk <[email protected]>
Co-authored-by: Aldo Gonzalez <[email protected]>
Co-authored-by: Dhroov Makwana <[email protected]>
Co-authored-by: jake horban <[email protected]>
Co-authored-by: Marcos Marx <[email protected]>
Co-authored-by: marcosmarxm <[email protected]>
Co-authored-by: btkcodedev <[email protected]>
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
Co-authored-by: Jonathan Pearlin <[email protected]>
Co-authored-by: Francis Genet <[email protected]>
Co-authored-by: Patrick Nilan <[email protected]>
Co-authored-by: Aldo Gonzalez <[email protected]>
Co-authored-by: Alfredo Garcia <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: suhl79 <[email protected]>
Co-authored-by: Satish Chinthanippu <[email protected]>
Co-authored-by: Sena Heydari <[email protected]>
Co-authored-by: Matt Bayley <[email protected]>
Co-authored-by: Edward Gao <[email protected]>
Co-authored-by: Matteo Palarchio <[email protected]>
Co-authored-by: Wenqi Hu <[email protected]>
Co-authored-by: Yue Li <[email protected]>
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.

3 participants