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

Support OAuth authentication for Big Query #431

Merged
merged 7 commits into from
Aug 2, 2023

Conversation

MonideepDe
Copy link
Contributor

Description

Support OAuth authentication for Big Query

Related Issue(s)

closes #420

Breaking Change?

None

Checklist

  • I have made corresponding changes to the documentation (if required)
  • I have added tests that prove my fix is effective or that my feature works

@MonideepDe MonideepDe requested a review from a team as a code owner August 1, 2023 23:35
@MonideepDe MonideepDe requested a review from a team August 1, 2023 23:35
@netlify
Copy link

netlify bot commented Aug 1, 2023

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

Name Link
🔨 Latest commit
🔍 Latest deploy log https://app.netlify.com/sites/amazing-pothos-a3bca0/deploys/64ca77b08136263a2e1b7c2b

@codecov
Copy link

codecov bot commented Aug 1, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -11.69% ⚠️

Comparison is base (dd553dc) 91.28% compared to head (51c80b9) 79.60%.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #431       +/-   ##
===========================================
- Coverage   91.28%   79.60%   -11.69%     
===========================================
  Files          48       49        +1     
  Lines        1699     1711       +12     
===========================================
- Hits         1551     1362      -189     
- Misses        148      349      +201     
Files Changed Coverage Δ
cosmos/profiles/__init__.py 96.15% <100.00%> (+0.15%) ⬆️
cosmos/profiles/base.py 95.78% <100.00%> (ø)
cosmos/profiles/bigquery/__init__.py 100.00% <100.00%> (ø)
cosmos/profiles/bigquery/oauth.py 100.00% <100.00%> (ø)

... and 11 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MonideepDe
Copy link
Contributor Author

MonideepDe commented Aug 2, 2023

@jlaneve The tests are failing with:

[2023-08-02 09:29:21,675] {graph.py:185} INFO - Skipping line:   Env var required but not provided: 'POSTGRES_HOST'

This PR does not alter anything related to POSTGRES_HOST. I can see the same issue in another PR's check as well.
I can see the errors when I run the tests against the main branch on my local as well 😞

Are you aware of a test bug of some sorts that has creeped in and if there is a PR already in the work for the same?

I hope I am missing something obvious 😢

Thanks!

@jlaneve
Copy link
Collaborator

jlaneve commented Aug 2, 2023

@jlaneve The tests are failing with:

[2023-08-02 09:29:21,675] {graph.py:185} INFO - Skipping line:   Env var required but not provided: 'POSTGRES_HOST'

This PR does not alter anything related to POSTGRES_HOST. I can see the same issue in another PR's check as well. I can see the errors when I run the tests against the main branch on my local as well 😞

Are you aware of a test bug of some sorts that has creeped in and if there is a PR already in the work for the same?

I hope I am missing something obvious 😢

Thanks!

Unfortunately this is a known issue when there are PRs from forked repos because CI secrets don’t get set. As long as the unit tests and pre-commit checks pass, we can merge this! It looks like there’s just one minor issue with the pre-commit checks - one of the lines is too long

Copy link
Collaborator

@jlaneve jlaneve left a comment

Choose a reason for hiding this comment

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

this looks great, thank you so much @MonideepDe!

@jlaneve jlaneve merged commit 8c49661 into astronomer:main Aug 2, 2023
22 of 39 checks passed
tatiana pushed a commit that referenced this pull request Aug 9, 2023
## Description

Support OAuth authentication for Big Query

## Related Issue(s)
closes #420 

## Breaking Change?

None

## Checklist

- [ ] I have made corresponding changes to the documentation (if
required)
- [X] I have added tests that prove my fix is effective or that my
feature works

---------

Co-authored-by: Monideep De <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
tatiana added a commit that referenced this pull request Aug 9, 2023
Enhancements
* Improve logs to include astornomer-cosmos identifier by @tatiana in #450
* Support OAuth authentication for Big Query by @MonideepDe in #431

Bug fixes
* Fix selector for config tags by @javihernovoa in #441
* Fix BigQuery keyfile_dict mapping for connection created from webserver UI by @jbandoro in #449

Others
* [pre-commit.ci] pre-commit autoupdate by @pre-commit-ci in #446
* Resolve MyPy errors when adding Airflow pre-commit dependency by @abhi12mohan in #434
@tatiana tatiana mentioned this pull request Aug 9, 2023
tatiana added a commit that referenced this pull request Aug 10, 2023
Enhancements
* Improve logs to include astronomer-cosmos identifier by @tatiana in
#450
* Support OAuth authentication for Big Query by @MonideepDe in #431
    
Bug fixes
* Fix selector for config tags by @javihernovoa in #441
* Fix BigQuery keyfile_dict mapping for connection created from
webserver UI by @jbandoro in #449
    
Others
* [pre-commit.ci] pre-commit autoupdate by @pre-commit-ci in #446
* Resolve MyPy errors when adding Airflow pre-commit dependency by @abhi12mohan in #434
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.

Support OAuth authentication for Big Query
2 participants