-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
Added Amazon SageMaker Notebook hook and operators #33219
Conversation
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some comments. Most of them are related to style/convention. Overall it is pretty good! Thanks for doing these changes.
tests/system/providers/amazon/aws/example_sagemaker_notebook.py
Outdated
Show resolved
Hide resolved
tests/system/providers/amazon/aws/example_sagemaker_notebook.py
Outdated
Show resolved
Hide resolved
tests/system/providers/amazon/aws/example_sagemaker_notebook.py
Outdated
Show resolved
Hide resolved
Updated the PR with the suggested changes, except the file location for the operator. Open to feedback on whether to integrate into the existing sageMaker file, or leave as-is. |
Integrating it into the existing SageMaker file makes sense to me ;) |
tests/providers/amazon/aws/operators/test_sagemaker_notebook.py
Outdated
Show resolved
Hide resolved
tests/system/providers/amazon/aws/example_sagemaker_notebook.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A last nit :) Thanks for your reactivity!
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Thanks for your work and reactivity! This is really good! I'll merge it once the tests are passing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
will merge when CI is green
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
--------- Co-authored-by: Vincent <97131062+vincbeck@users.noreply.github.com>
Closes: #32702
Created new hook and Operators for Amazon SageMaker Notebooks Instances. Allows user to create, start, stop, and delete notebook instances.
Created unit tests for hook and operators and system test. All tests passed.
Updated documentation for new operator.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rst
or{issue_number}.significant.rst
, in newsfragments.