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

Added RDS & S3 Credentials Setup For Notebooks & Pipeline #106

Merged
merged 1 commit into from
Mar 16, 2022
Merged

Added RDS & S3 Credentials Setup For Notebooks & Pipeline #106

merged 1 commit into from
Mar 16, 2022

Conversation

AlexandreBrown
Copy link
Contributor

@AlexandreBrown AlexandreBrown commented Feb 28, 2022 •
edited
Loading

Which issue is resolved by this Pull Request:
Resolves #100

Description of your changes:
This PR adds instructions on how and when to setup RDS & S3 Credentials for Notebooks & Pipeline.

2022-03-04_19-55-00.mp4

Checklist:

  • Unit tests pass:
    Make sure you have installed kustomize == 3.2.1
    1. make generate-changed-only
    2. make test
  • End-to-end test on fresh install (main branch + manifest v1.4.1)
  • Support S3 credentials
  • Support DB credentials

@AlexandreBrown AlexandreBrown changed the title Feature/100 Added PodDefault setup instructions Feb 28, 2022
@AlexandreBrown AlexandreBrown force-pushed the feature/100 branch 2 times, most recently from 838c3ff to db3ae05 Compare February 28, 2022 22:10
goswamig
goswamig reviewed Feb 28, 2022
View reviewed changes
Copy link
Member

@goswamig goswamig left a comment

Choose a reason for hiding this comment

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

Great work!!!

Minor comment.

@AlexandreBrown AlexandreBrown changed the title Added PodDefault setup instructions WIP (Help needed): Added PodDefault setup instructions Mar 1, 2022
@AlexandreBrown AlexandreBrown changed the title WIP (Help needed): Added PodDefault setup instructions WIP (Feedback needed): Added PodDefault setup instructions Mar 2, 2022
@AlexandreBrown AlexandreBrown changed the title WIP (Feedback needed): Added PodDefault setup instructions [WIP] (Feedback needed): Added PodDefault setup instructions Mar 2, 2022
@AlexandreBrown AlexandreBrown changed the title [WIP] (Feedback needed): Added PodDefault setup instructions [WIP] : Added PodDefault setup instructions Mar 4, 2022
@AlexandreBrown AlexandreBrown changed the title [WIP] : Added PodDefault setup instructions Added RDS & S3 Credentials Setup For Notebooks & Pipeline Mar 5, 2022
@AlexandreBrown AlexandreBrown force-pushed the feature/100 branch 4 times, most recently from 098d41d to e4a2aa5 Compare March 5, 2022 01:03
@AlexandreBrown
Copy link
Contributor Author

AlexandreBrown commented Mar 5, 2022 •
edited
Loading

PR is ready for review, no longer WIP. @goswamig @surajkota
I tested it on 3 different fresh test clusters using main branch + v1.4.1 upstrean.

surajkota
surajkota requested changes Mar 8, 2022
View reviewed changes
Copy link
Contributor

@surajkota surajkota left a comment

Choose a reason for hiding this comment

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

Thanks for the investigation. Added a few comments for improving documentation and scripts

docs/deployment/rds-s3/secrets/secrets-setup.py Outdated Show resolved Hide resolved
docs/deployment/rds-s3/secrets/secrets-setup.py Outdated Show resolved Hide resolved
docs/deployment/rds-s3/secrets/secrets-setup.py Outdated Show resolved Hide resolved
docs/deployment/rds-s3/secrets/secrets-setup.py Outdated Show resolved Hide resolved
docs/deployment/rds-s3/secrets/secrets-setup.py Outdated Show resolved Hide resolved
docs/deployment/rds-s3/secrets/secrets-setup.py Outdated Show resolved Hide resolved
docs/deployment/rds-s3/README.md Outdated Show resolved Hide resolved
docs/deployment/rds-s3/README.md Outdated Show resolved Hide resolved
docs/deployment/rds-s3/README.md Outdated Show resolved Hide resolved
docs/deployment/rds-s3/README.md Outdated Show resolved Hide resolved
@AlexandreBrown AlexandreBrown force-pushed the feature/100 branch 3 times, most recently from caad0f6 to 987a098 Compare March 9, 2022 20:39
@AlexandreBrown
Copy link
Contributor Author

Ping @surajkota @goswamig

surajkota
surajkota requested changes Mar 14, 2022
View reviewed changes
Copy link
Contributor

@surajkota surajkota left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
Requesting a few cosmetic changes

tests/e2e/utils/notebooks/common.py Outdated Show resolved Hide resolved
tests/e2e/utils/notebooks/setup_secrets_access.py Outdated Show resolved Hide resolved
tests/e2e/utils/notebooks/setup_secrets_access.py Outdated Show resolved Hide resolved
docs/deployment/rds-s3/README.md Outdated Show resolved Hide resolved
docs/component-guides/notebooks.md Outdated Show resolved Hide resolved
docs/component-guides/notebooks.md Show resolved Hide resolved
@surajkota surajkota mentioned this pull request Mar 15, 2022
Closed
31 tasks
@AlexandreBrown AlexandreBrown force-pushed the feature/100 branch 2 times, most recently from 701b93a to 4170afe Compare March 16, 2022 18:09
surajkota
surajkota requested changes Mar 16, 2022
View reviewed changes
Copy link
Contributor

@surajkota surajkota left a comment •
edited
Loading

Choose a reason for hiding this comment

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

thanks for taking care of all the comments, requesting final change

awsconfigs/apps/jupyter-web-app/configs/pod-default.yaml Outdated Show resolved Hide resolved
docs/component-guides/notebooks.md Show resolved Hide resolved
@surajkota surajkota merged commit 0b46bdb into awslabs:main Mar 16, 2022
@AlexandreBrown AlexandreBrown deleted the feature/100 branch March 16, 2022 23:16
@surajkota surajkota mentioned this pull request Mar 17, 2022
Merged
2 tasks
@surajkota surajkota mentioned this pull request Apr 19, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@goswamig goswamig goswamig approved these changes

@surajkota surajkota surajkota approved these changes

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

Successfully merging this pull request may close these issues.

Facilitate configuration of RDS & S3 credentials access for Notebooks & Pipelines
3 participants
@AlexandreBrown @goswamig @surajkota