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

Make BigQuery SqlAlchemy dialect conform to "dialect" attribute convention and dataset cleanup #3259

Conversation

jdimatteo
Copy link
Contributor

Changes proposed in this pull request:

  • Make BigQuery SqlAlchemy dialect conform to "dialect" attribute convention
  • remove duplicate inconsistent logic around BigQuery dataset used in tests
  • document how to override the BigQuery dataset name used in tests

@jdimatteo
Copy link
Contributor Author

jdimatteo commented Aug 16, 2021

Note that I confirmed pytest tests/test_definitions/test_expectations_cfe.py --bigquery --no-spark --no-postgresql passed for me locally.

I also ran tests/test_definitions/test_expectations.py but that hasn't finished running yet and that passed too.

Comment on lines +101 to +104
if not hasattr(pybigquery.sqlalchemy_bigquery, "dialect"):
pybigquery.sqlalchemy_bigquery.dialect = (
pybigquery.sqlalchemy_bigquery.BigQueryDialect
)
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@jdimatteo jdimatteo marked this pull request as ready for review August 17, 2021 03:53
…' into feature/GREAT-66/enable-bigquery-tests-for-expectations
Copy link
Contributor

@Shinnnyshinshin Shinnnyshinshin left a comment

Choose a reason for hiding this comment

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

Wonderful. Thank you very very much @jdimatteo

@Shinnnyshinshin Shinnnyshinshin merged commit 637da06 into great-expectations:feature/GREAT-66/enable-bigquery-tests-for-expectations Aug 17, 2021
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.

None yet

3 participants