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

Refactor assume_schema_exists and expose it in aql.transform #1925

Merged
merged 9 commits into from
May 5, 2023

Conversation

tatiana
Copy link
Collaborator

@tatiana tatiana commented May 5, 2023

This is a follow-up for #1922. In that PR we allowed users to skip schema check & creation for aql.load_file, but we missed the fact that aql.transform and aql.transform_file had the same issue. This PR aims to address this limitation.

Changes included in this PR:

  • Rename config load_table_schema_exists to assume_schema_exists
  • Rename (load_file) argument schema_exists to assume_schema_exists
  • Refactor where the check for assume_schema_exists happens. Before, it happened only inside the load_file_to_table. Now, it is part of create_schema_if_applicable. This makes this feature available in the aql.transform task as well
  • Rename Database.create_schema_if_needed to Database.create_schema_if_applicable
  • Expose assume_schema_exists in aql.transform
  • Release 1.7.0a2

@tatiana tatiana changed the title Refactor config load_table_schema_exists to assume_schema_exists Refactor load_table_schema_exists to assume_schema_exists and expose it in transform May 5, 2023
@tatiana tatiana changed the title Refactor load_table_schema_exists to assume_schema_exists and expose it in transform Refactor config to assume_schema_exists and expose it in transform May 5, 2023
@tatiana tatiana changed the title Refactor config to assume_schema_exists and expose it in transform Refactor assume_schema_exists and expose it in transform May 5, 2023
@codecov
Copy link

codecov bot commented May 5, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +7.48 🎉

Comparison is base (55b7a8b) 84.81% compared to head (ca57b34) 92.29%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1925      +/-   ##
==========================================
+ Coverage   84.81%   92.29%   +7.48%     
==========================================
  Files         104       72      -32     
  Lines        5959     4246    -1713     
  Branches      678      512     -166     
==========================================
- Hits         5054     3919    -1135     
+ Misses        762      235     -527     
+ Partials      143       92      -51     
Flag Coverage Δ
PythonSDK 92.29% <100.00%> (-0.07%) ⬇️
UTO ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
python-sdk/src/astro/__init__.py 100.00% <100.00%> (ø)
python-sdk/src/astro/databases/base.py 91.83% <100.00%> (-0.04%) ⬇️
python-sdk/src/astro/databases/databricks/delta.py 85.71% <100.00%> (ø)
python-sdk/src/astro/databases/postgres.py 96.73% <100.00%> (ø)
python-sdk/src/astro/settings.py 100.00% <100.00%> (ø)
python-sdk/src/astro/sql/operators/load_file.py 98.21% <100.00%> (ø)
python-sdk/src/astro/sql/operators/transform.py 90.62% <100.00%> (+0.62%) ⬆️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tatiana tatiana marked this pull request as ready for review May 5, 2023 22:20
@tatiana tatiana force-pushed the assume-schema-exists branch from 30e8eb9 to ca57b34 Compare May 5, 2023 22:41
@tatiana tatiana requested a review from kaxil May 5, 2023 22:41
@tatiana tatiana changed the title Refactor assume_schema_exists and expose it in transform Refactor assume_schema_exists and expose it in aql.transform May 5, 2023
@tatiana tatiana added this to the 1.7.0 milestone May 5, 2023
@tatiana tatiana merged commit 21e8d47 into main May 5, 2023
@tatiana tatiana deleted the assume-schema-exists branch May 5, 2023 23:08
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.

2 participants