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.

Sign up for GitHub

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

Jump to bottom

Implement Async Big Query Table Existence Sensor #135

Merged
merged 10 commits into from
Mar 23, 2022
Merged

Implement Async Big Query Table Existence Sensor #135

merged 10 commits into from
Mar 23, 2022

Conversation

bharanidharan14
Copy link
Contributor

story Id: #68

  • Implemented Async Big Query Table Existence Sensor (Operator, Trigger, Hook) --> @rajaths010494
  • Added Unit Test case, code coverage, Doc string For Big Query Table Existence Sensor, code coverage --> @bharanidharan14

rajaths010494 and others added 4 commits March 14, 2022 17:04
@rajaths010494
Implement BigQueryTableExistenceSensorAsync
e12cf00 
- Implement BigQueryTableExistenceSensorAsync
- Implement BigQueryTableExistenceTrigger
- Implement BigQueryTableHookAsync
@rajaths010494
Fix static checks failed in CI
80ea1a9
@rajaths010494
Fix mypy issue from CI and add catch ClientResponseError
fbfac13
@bharanidharan14
Add Test case For BigQuery Table Existences Sensor
427ac44 
Add Test case For BigQuery Table Existences Sensor

- Add Test case
- corrected Doc string
@bharanidharan14
Change connection id
064e040 
- Add GCP_CONN_ID as env variables
- Change 'google_cloud_conn_id' to 'gcp_conn_id'
- Fix doc string
@bharanidharan14
Update test_bigquery.py
67001a2 
- Fixed connection id
@bharanidharan14
Merge branch 'main' into big-query-table-sensor
097a226
@codecov
Copy link

codecov bot commented Mar 17, 2022 •
edited
Loading

Codecov Report

Merging #135 (f39b462) into main (896b9de) will increase coverage by 0.13%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #135      +/-   ##
==========================================
+ Coverage   95.34%   95.48%   +0.13%     
==========================================
  Files          35       36       +1     
  Lines        1954     2014      +60     
==========================================
+ Hits         1863     1923      +60     
  Misses         91       91
Impacted Files Coverage Δ
...stronomer/providers/google/cloud/hooks/bigquery.py 99.22% <100.00%> (+0.03%) ⬆️
...ronomer/providers/google/cloud/sensors/bigquery.py 100.00% <100.00%> (ø)
...onomer/providers/google/cloud/triggers/bigquery.py 97.93% <100.00%> (+0.46%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 896b9de...f39b462. Read the comment docs.

sunank200
sunank200 reviewed Mar 17, 2022
View reviewed changes
Copy link
Collaborator

@sunank200 sunank200 left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Just few minor comments.

astronomer/providers/google/cloud/example_dags/example_bigquery_sensors.py Show resolved Hide resolved
astronomer/providers/google/cloud/hooks/bigquery.py Outdated Show resolved Hide resolved
astronomer/providers/google/cloud/triggers/bigquery.py Outdated Show resolved Hide resolved
@bharanidharan14
Add Doc string
afbccdb 
- Added Doc string
- Removed super().__init__()
@bharanidharan14 bharanidharan14 marked this pull request as ready for review March 17, 2022 11:06
@bharanidharan14
Copy link
Contributor Author

@phanikumv Fixed as per review comment

@phanikumv phanikumv merged commit 10364b7 into main Mar 23, 2022
@phanikumv phanikumv deleted the big-query-table-sensor branch March 23, 2022 07:17
@kaxil kaxil mentioned this pull request Mar 23, 2022
Merged
@kaxil
Copy link
Collaborator

kaxil commented Mar 23, 2022

Let's make sure while merging PR's we edit/make sure that the Commit Message and description contain summarized text, for example for this commit it contains:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rajaths010494 rajaths010494 rajaths010494 approved these changes

@phanikumv phanikumv Awaiting requested review from phanikumv phanikumv is a code owner

@kaxil kaxil Awaiting requested review from kaxil

@sunank200 sunank200 Awaiting requested review from sunank200 sunank200 is a code owner

@pankajastro pankajastro Awaiting requested review from pankajastro pankajastro is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@bharanidharan14 @kaxil @sunank200 @rajaths010494 @phanikumv @pankajastro