Skip to content

Implement Async GCS object update sensor #93

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
Merged
merged 29 commits into from
Mar 10, 2022
Merged

Implement Async GCS object update sensor #93

merged 29 commits into from
Mar 10, 2022

Conversation

bharanidharan14
Copy link
Contributor

@bharanidharan14 bharanidharan14 commented Mar 3, 2022 •
edited
Loading

Story Id: #69

  • Implemented Async Operator and Trigger for GCSObjectUpdateSensorAsync

rajaths010494 and others added 10 commits March 2, 2022 19:04
@rajaths010494
Implement GCSObjectWithPrefixExistenceSensor
0c6f7e4 
- Implement the GCSPrefixBlobTrigger(
- Implement the sensor GCSObjectWithPrefixExistenceSensor
@rajaths010494
Fix static check in triggers
e4f5814
@rajaths010494
add example dag for GCSObjectsWithPrefixExistenceSensorAsync
22e22da 
- add example dag for GCSObjectsWithPrefixExistenceSensorAsync
- modify trigger super.__init__
@rajaths010494
Add tests for GCSObjectsWithPrefixExistenceSensorAsync
a5aa480
@rajaths010494
Fix mypy issues in gcs sensor
6207214
@bharanidharan14
Implement Async GCSObjectUpdateSensorAsync
027dec9 
-  Add GCSObjectUpdateSensorAsync  Operator, Trigger
@bharanidharan14
Fix Test Coverage Snowflake and Redshift Cluster
4307ebb 
- Fix Snowflake Trigger, Operator, Hooks code coverage above 92
- Fix Redshift Cluster Trigger, Operator code coverage above 96
@bharanidharan14
Merge branch 'main' into Fix_code_coverage
2cc560a
@bharanidharan14
Merge branch 'main' into GCS_ObjectUpdateSensor
88a9424
@bharanidharan14
Add Test case and Add GCSObjectUpdateSensorAsync example operator
6f8f1e8
sunank200
sunank200 requested changes Mar 7, 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.

Added few comments and few changes.

One question - Is there a specific reason to keep certain methods as private methods in triggers rather than keeping in hooks?

@rajaths010494 rajaths010494 self-requested a review March 7, 2022 12:32
pankajastro
pankajastro reviewed Mar 8, 2022
View reviewed changes
@bharanidharan14 bharanidharan14 marked this pull request as ready for review March 9, 2022 12:14
kaxil
kaxil reviewed Mar 9, 2022
View reviewed changes
kaxil
kaxil reviewed Mar 9, 2022
View reviewed changes
kaxil
kaxil reviewed Mar 9, 2022
View reviewed changes
kaxil
kaxil reviewed Mar 9, 2022
View reviewed changes
kaxil
kaxil reviewed Mar 9, 2022
View reviewed changes
kaxil
kaxil reviewed Mar 9, 2022
View reviewed changes
kaxil
kaxil reviewed Mar 9, 2022
View reviewed changes
@bharanidharan14
Fix in Snowflake hook, GCS
a15f501 
- Add back the else condition and return value in hook along with test case covered
- Add proper doc string
- Remove unwanted print
- Remove wired formatting for query and added test query with proper query
@bharanidharan14 bharanidharan14 requested a review from kaxil March 9, 2022 16:43
@bharanidharan14 bharanidharan14 changed the title GCS object update sensor, Code Coverage #87, mypy fix implemented GCS object update sensor, FIx Code Coverage #87, Fix mypy issues Mar 9, 2022
@bharanidharan14 bharanidharan14 changed the title implemented GCS object update sensor, FIx Code Coverage #87, Fix mypy issues Implemented GCS object update sensor, FIx Code Coverage #87, Fix mypy issues Mar 9, 2022
@bharanidharan14 bharanidharan14 changed the title Implemented GCS object update sensor, FIx Code Coverage #87, Fix mypy issues Implemented GCS object update sensor, Fix Code Coverage #87, Fix mypy issues Mar 9, 2022
@bharanidharan14 bharanidharan14 changed the title Implemented GCS object update sensor, Fix Code Coverage #87, Fix mypy issues Implement GCS object update sensor, Fix Code Coverage #87, Fix mypy issues Mar 9, 2022
kaxil
kaxil reviewed Mar 9, 2022
View reviewed changes
kaxil
kaxil reviewed Mar 9, 2022
View reviewed changes
Copy link
Collaborator

@kaxil kaxil left a comment

Choose a reason for hiding this comment

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

Please separate these into at least 2 or 3 PRs.

  1. Implements Async GCS Object Update Sensor
  2. Fixes Code Coverage
  3. Fixes MyPy issues

@bharanidharan14 bharanidharan14 changed the title Implement GCS object update sensor, Fix Code Coverage #87, Fix mypy issues Implement GCS object update sensor Mar 10, 2022
@bharanidharan14 bharanidharan14 requested a review from kaxil March 10, 2022 07:12
@bharanidharan14 bharanidharan14 changed the title Implement GCS object update sensor Implement Async GCS object update sensor Mar 10, 2022
@bharanidharan14
Copy link
Contributor Author

Please separate these into at least 2 or 3 PRs.

  1. Implements Async GCS Object Update Sensor
  2. Fixes Code Coverage
  3. Fixes MyPy issues

Added Two PR

  1. Implements Async GCS Object Update Sensor--> eb31453
  2. Fix Code coverage--> Fix Code coverage #107

@bharanidharan14
Copy link
Contributor Author

Added few comments and few changes.

One question - Is there a specific reason to keep certain methods as private methods in triggers rather than keeping in hooks?

I was following the existing implementation

phanikumv
phanikumv approved these changes Mar 10, 2022
View reviewed changes
Copy link
Collaborator

@phanikumv phanikumv left a comment

Choose a reason for hiding this comment

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

Looks good to me

@phanikumv
Copy link
Collaborator

@kaxil @sunank200 are we good to merge this?

sunank200
sunank200 approved these changes Mar 10, 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.

LGTM

@kaxil kaxil merged commit c471fc1 into main Mar 10, 2022
@kaxil kaxil deleted the GCS_ObjectUpdateSensor branch March 10, 2022 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sunank200 sunank200 sunank200 approved these changes

@kaxil kaxil kaxil approved these changes

@phanikumv phanikumv phanikumv approved these changes

@rajaths010494 rajaths010494 Awaiting requested review from rajaths010494

@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 @phanikumv @sunank200 @kaxil @pankajastro @rajaths010494