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

Update query for getting bigquery table schema #661

Merged
merged 2 commits into from
Aug 16, 2022
Merged

Conversation

utkarsharma2
Copy link
Collaborator

@utkarsharma2 utkarsharma2 commented Aug 16, 2022

Description

What is the current behavior?

There is a failing test case in CI - test_bigquery_create_table_with_columns

Can be because of PR - #641 (comment)
Action run failure - https://github.com/astronomer/astro-sdk/runs/7830499547?check_suite_focus=true

Astro: 1.0.0b1

closes: #660

What is the new behavior?

Bigquery added an extra col in the INFORMATION_SCHEMA.COLUMNS and the test broke in CI since we checked for the entire cols involved. Now we are only looking for specific columns, this should make the test more robust.

Does this introduce a breaking change?

Nope

Checklist

  • Created tests that fail without the change (if possible)

@utkarsharma2 utkarsharma2 changed the title Updat query for bigquery schema from to specfic cols Update query for bigquery schema from to specfic cols Aug 16, 2022
@utkarsharma2 utkarsharma2 changed the title Update query for bigquery schema from to specfic cols Update query for getting bigquery table schema Aug 16, 2022
Copy link
Member

@feluelle feluelle left a comment

Choose a reason for hiding this comment

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

I think it is good practice that you did specify the columns now. 👍

tests/databases/test_bigquery.py Outdated Show resolved Hide resolved
statement = f"SELECT * FROM {table.metadata.schema}.INFORMATION_SCHEMA.COLUMNS WHERE table_name='{table.name}'"
# Looking for specific columns in INFORMATION_SCHEMA.COLUMNS as Bigquery can add/remove columns in the table.
statement = (
f"SELECT TABLE_CATALOG, TABLE_SCHEMA, TABLE_NAME, COLUMN_NAME, DATA_TYPE FROM {table.metadata.schema}"
Copy link
Contributor

@sunank200 sunank200 Aug 16, 2022

Choose a reason for hiding this comment

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

What are those extra columns btw? This looks good to be but it could be replaced with the values on this assert: https://github.com/astronomer/astro-sdk/runs/7830499547?check_suite_focus=true#step:11:765

Also I think f-string change has to be done for deepsource

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@sunank200 column_default is the col added. Which is not listed in https://cloud.google.com/bigquery/docs/information-schema-columns

Co-authored-by: Felix Uellendall <feluelle@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Aug 16, 2022

Codecov Report

Merging #661 (d12f436) into main (905f214) will decrease coverage by 0.52%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #661      +/-   ##
==========================================
- Coverage   93.29%   92.76%   -0.53%     
==========================================
  Files          41       42       +1     
  Lines        1670     1701      +31     
  Branches      210      216       +6     
==========================================
+ Hits         1558     1578      +20     
- Misses         89       97       +8     
- Partials       23       26       +3     
Impacted Files Coverage Δ
src/astro/sql/operators/raw_sql.py 82.35% <0.00%> (-17.65%) ⬇️
src/astro/sql/operators/transform.py 82.35% <0.00%> (-17.65%) ⬇️
src/astro/sql/operators/drop.py 90.00% <0.00%> (-10.00%) ⬇️
src/astro/sql/operators/append.py 92.85% <0.00%> (-7.15%) ⬇️
src/astro/sql/operators/merge.py 93.54% <0.00%> (-6.46%) ⬇️
src/astro/sql/operators/dataframe.py 92.95% <0.00%> (-3.82%) ⬇️
src/astro/sql/operators/load_file.py 94.28% <0.00%> (-2.82%) ⬇️
src/astro/databases/base.py 96.07% <0.00%> (-0.06%) ⬇️
src/astro/databases/google/bigquery.py 97.41% <0.00%> (-0.02%) ⬇️
src/astro/exceptions.py 100.00% <0.00%> (ø)
... and 6 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@pankajkoti pankajkoti left a comment

Choose a reason for hiding this comment

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

The Deepsource:Python analysis is suggesting that Line 446 in this file can be converted to f-string. It is not part of your change but can we convert the format clause string to an f-string. https://github.com/astronomer/astro-sdk/pull/661/files#diff-d5242436b93ad662a1ef1f5df4b89b9c3da692fb988142b475fc72c1ddfdf7beR446

Analysis message: https://deepsource.io/gh/astronomer/astro-sdk/run/2044361d-845a-43fc-b418-c1da18b66adb/python/PYL-C0209

@utkarsharma2
Copy link
Collaborator Author

@kaxil I'm merging this as it will be a blocker for other PRs. Please let me know if you think there should be some changes here.

@utkarsharma2
Copy link
Collaborator Author

@pankajkoti I'll raise a separate PR for this, as the changes are not related to the current issue.

@utkarsharma2 utkarsharma2 merged commit a80d953 into main Aug 16, 2022
@utkarsharma2 utkarsharma2 deleted the FixBigqueryTests branch August 16, 2022 08:15
kaxil pushed a commit that referenced this pull request Aug 18, 2022
* Updat query for bigquery schema from  to specfic cols

* Update tests/databases/test_bigquery.py

Co-authored-by: Felix Uellendall <feluelle@users.noreply.github.com>a
Description
What is the current behavior?

There is a failing test case in CI - test_bigquery_create_table_with_columns

Can be because of PR - #641 (comment)
Action run failure - https://github.com/astronomer/astro-sdk/runs/7830499547?check_suite_focus=true

Astro: 1.0.0b1

closes: #660
What is the new behavior?

Bigquery added an extra col in the INFORMATION_SCHEMA.COLUMNS and the test broke in CI since we checked for the entire cols involved. Now we are only looking for specific columns, this should make the test more robust.
Does this introduce a breaking change?

Nope
Checklist

    Created tests that fail without the change (if possible)
feluelle pushed a commit that referenced this pull request Aug 19, 2022
* Updat query for bigquery schema from  to specfic cols

* Update tests/databases/test_bigquery.py

Co-authored-by: Felix Uellendall <feluelle@users.noreply.github.com>a
Description
What is the current behavior?

There is a failing test case in CI - test_bigquery_create_table_with_columns

Can be because of PR - #641 (comment)
Action run failure - https://github.com/astronomer/astro-sdk/runs/7830499547?check_suite_focus=true

Astro: 1.0.0b1

closes: #660
What is the new behavior?

Bigquery added an extra col in the INFORMATION_SCHEMA.COLUMNS and the test broke in CI since we checked for the entire cols involved. Now we are only looking for specific columns, this should make the test more robust.
Does this introduce a breaking change?

Nope
Checklist

    Created tests that fail without the change (if possible)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate test Case in CI is failing - test_bigquery_create_table_with_columns
5 participants