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

fix(source-google-sheets): add sheet id encoding #52671

Merged
merged 79 commits into from
Jan 31, 2025

Conversation

lazebnyi
Copy link
Collaborator

@lazebnyi lazebnyi commented Jan 31, 2025

What

In the connector, query parameters are passed directly in the path and they are not encoded. Therefore, when the sheet ID (sheet name) contains special characters, the URL may not be resolved correctly.

Example:

Old version:
Sheet1 one col&special name%? -> range=Sheet1%20one%20col&special%20name%?!1:1

New version:
Sheet1 one col&special name%? -> ranges=Sheet1%20one%20col%26special%20name%25%3F!1:1

How

Add urlencode to path resolving

User Impact

No

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

aldogonzalez8 and others added 30 commits December 12, 2024 11:33
…xtractor that matches schema properties with values
…in components resolver in order to get an indexed object of properties where index is the order.

 - Add partition router to retriver in components resolver so we can slice.
 - Remove old SourceClass to temp file to be deleted and

 - Remove unused transformations

 - Make extractor ha transform data from component mapping
Copy link

vercel bot commented Jan 31, 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 Jan 31, 2025 0:32am

@octavia-squidington-iii octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/google-sheets labels Jan 31, 2025
…ub.com:airbytehq/airbyte into lazebnyi/fix-sheed-id-encode-for-google-sheets
@lazebnyi
Copy link
Collaborator Author

Regression tests looks good - Job 1

Only test_catalog_are_the_same by same reason - #50843 (comment).

Copy link
Contributor

@maxi297 maxi297 left a comment

Choose a reason for hiding this comment

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

LGTM! I'm fine with this being covered by CATs as well

@lazebnyi lazebnyi merged commit 9130c9f into master Jan 31, 2025
31 of 32 checks passed
@lazebnyi lazebnyi deleted the lazebnyi/fix-sheed-id-encode-for-google-sheets branch January 31, 2025 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/google-sheets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants