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 Doc string for SnowflakeOperatorAsync #556

Merged
merged 1 commit into from
Aug 1, 2022

Conversation

bharanidharan14
Copy link
Contributor

@bharanidharan14 bharanidharan14 commented Jul 28, 2022

Added Doc string for the SnowflakeOperatorAsync to get to know the use of this operator compared with snowflake sql API operator, best practices to be followed while using this operator and where can this SnowflakeOperatorAsync can be used

closes: #557
Screenshot 2022-07-28 at 2 56 48 PM
Screenshot 2022-07-28 at 5 56 44 PM

@codecov
Copy link

codecov bot commented Jul 28, 2022

Codecov Report

Merging #556 (d477832) into main (46a607f) will not change coverage.
The diff coverage is n/a.

❗ Current head d477832 differs from pull request most recent head b389902. Consider uploading reports for the commit b389902 to get more accurate results

@@           Coverage Diff           @@
##             main     #556   +/-   ##
=======================================
  Coverage   98.27%   98.27%           
=======================================
  Files          72       72           
  Lines        3996     3996           
=======================================
  Hits         3927     3927           
  Misses         69       69           
Impacted Files Coverage Δ
...ronomer/providers/snowflake/operators/snowflake.py 100.00% <ø> (ø)

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 46a607f...b389902. Read the comment docs.

@bharanidharan14
Copy link
Contributor Author

@phanikumv @rajaths010494 @pankajastro @pankajkoti, can you review this PR

@pankajastro
Copy link
Contributor

@phanikumv @rajaths010494 @pankajastro @pankajkoti, can you review this PR

Can you please post the screenshot like how it is looking you can run make docs?

@rajaths010494
Copy link
Contributor

LGTM , once Screenshot is added we can close.

@bharanidharan14
Copy link
Contributor Author

@rajaths010494 @pankajastro Added screenshot

@pankajastro
Copy link
Contributor

@rajaths010494 @pankajastro Added screenshot

I can't see the below text in the screenshot but it is in the doc string. I was curious to see how it looks otherwise changes look good to me

for the query status based on the query id from different connection.
.. see also::
https://docs.snowflake.com/en/user-guide/python-connector-example.html#label-python-connector-querying-data-asynchronous.

@bharanidharan14
Copy link
Contributor Author

@rajaths010494 @pankajastro Added screenshot

I can't see the below text in the screenshot but it is in the doc string. I was curious to see how it looks otherwise changes look good to me

for the query status based on the query id from different connection.
.. see also::
https://docs.snowflake.com/en/user-guide/python-connector-example.html#label-python-connector-querying-data-asynchronous.

See also link is not visible in Docs, its not showing up in any of the operators/sensors

@pankajastro
Copy link
Contributor

@rajaths010494 @pankajastro Added screenshot

I can't see the below text in the screenshot but it is in the doc string. I was curious to see how it looks otherwise changes look good to me

for the query status based on the query id from different connection.
.. see also::
https://docs.snowflake.com/en/user-guide/python-connector-example.html#label-python-connector-querying-data-asynchronous.

See also link is not visible in Docs, its not showing up in any of the operators/sensors

then this is a bug in our documentation please create a ticket to track it. But I was able to do it in #464

@bharanidharan14
Copy link
Contributor Author

@rajaths010494 @pankajastro Added screenshot

I can't see the below text in the screenshot but it is in the doc string. I was curious to see how it looks otherwise changes look good to me

for the query status based on the query id from different connection.
.. see also::
https://docs.snowflake.com/en/user-guide/python-connector-example.html#label-python-connector-querying-data-asynchronous.

See also link is not visible in Docs, its not showing up in any of the operators/sensors

then this is a bug in our documentation please create a ticket to track it. But I was able to do it in #464

Created Ticket

astronomer/providers/snowflake/operators/snowflake.py Outdated Show resolved Hide resolved
astronomer/providers/snowflake/operators/snowflake.py Outdated Show resolved Hide resolved
astronomer/providers/snowflake/operators/snowflake.py Outdated Show resolved Hide resolved
astronomer/providers/snowflake/operators/snowflake.py Outdated Show resolved Hide resolved
astronomer/providers/snowflake/operators/snowflake.py Outdated Show resolved Hide resolved
astronomer/providers/snowflake/operators/snowflake.py Outdated Show resolved Hide resolved
astronomer/providers/snowflake/operators/snowflake.py Outdated Show resolved Hide resolved
@bharanidharan14
Copy link
Contributor Author

@phanikumv Addressed your PR review changes

@bharanidharan14 bharanidharan14 force-pushed the snowflake-doc-fix branch 2 times, most recently from 28feeb3 to e819d86 Compare July 28, 2022 14:36
Copy link
Collaborator

@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.

LGTM, minor suggestions.

astronomer/providers/snowflake/operators/snowflake.py Outdated Show resolved Hide resolved
astronomer/providers/snowflake/operators/snowflake.py Outdated Show resolved Hide resolved
astronomer/providers/snowflake/operators/snowflake.py Outdated Show resolved Hide resolved
astronomer/providers/snowflake/operators/snowflake.py Outdated Show resolved Hide resolved
astronomer/providers/snowflake/operators/snowflake.py Outdated Show resolved Hide resolved
astronomer/providers/snowflake/operators/snowflake.py Outdated Show resolved Hide resolved
Update Doc string for SnowflakeOperatorAsync

Update Doc string for SnowflakeOperatorAsync

Doc fix

Fix flake8 issue

Doc string fix

Doc fix
@bharanidharan14
Copy link
Contributor Author

@phanikumv Addressed your review comments, can you please review it again

@bharanidharan14
Copy link
Contributor Author

@phanikumv Need your approval

@bharanidharan14 bharanidharan14 merged commit 0948767 into main Aug 1, 2022
@bharanidharan14 bharanidharan14 deleted the snowflake-doc-fix branch August 1, 2022 06:25
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.

Update Doc string for SnowflakeOperatorAsync
5 participants