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

LBED: zETL Opensearch lab #107

Merged
merged 33 commits into from
Mar 29, 2024
Merged

LBED: zETL Opensearch lab #107

merged 33 commits into from
Mar 29, 2024

Conversation

terhunej
Copy link
Contributor

Issue #, if available:

Description of changes:
Added new lab for DynamoDB zero-ETL to OpenSearch integration.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

tebanieo
tebanieo requested changes Feb 28, 2024
View reviewed changes
Copy link
Contributor

@tebanieo tebanieo left a comment

Choose a reason for hiding this comment

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

Hello, This lab looks very nice. I have added a couple of comments on the CloudFormation file and the minor formatting of the pictures, please ensure you add an extra "Enter" between pictures so the lab looks less cluttered.

content/hands-on-labs/dynamodb-opensearch-zetl/ddb-os-zetl-chapter02.en.md Outdated Show resolved Hide resolved
static/files/hands-on-labs/dynamodb-opensearch-setup.yaml Outdated Show resolved Hide resolved
static/files/hands-on-labs/dynamodb-opensearch-setup.yaml Outdated Show resolved Hide resolved
static/files/hands-on-labs/dynamodb-opensearch-setup.yaml Outdated Show resolved Hide resolved
static/files/hands-on-labs/dynamodb-opensearch-setup.yaml Outdated Show resolved Hide resolved
content/hands-on-labs/dynamodb-opensearch-zetl/ddb-os-zetl-chapter08.en.md Outdated Show resolved Hide resolved
static/images/ddb-os-zetl7.jpg Outdated Show resolved Hide resolved
@tebanieo
Copy link
Contributor

Oh, please rebase this branch as there were additional deployments. Thanks!

@amdhing
Copy link
Contributor

amdhing commented Mar 1, 2024

In addition to @tebanieo comments could you also add
1/ A warning note on the first page of the lab with a list of AWS services that will be used as part of this lab and an approximate of costs that may be incurred ("10s to 100s of dollars per day" will do if appropriate) if the reader were following the lab in their own AWS accounts (and not AWS provided lab accounts).

2/ A warning note on the final "Query and Conclusion" page reminding reader to delete the CFN stack and resources created as part of the lab if they were following it in their own AWS accounts. Can refer to https://catalog.workshops.aws/dynamodb-labs/en-US/game-player-data/summary for the same.

Thanks!

@terhunej
Merge branch 'aws-samples:master' into opensearch-lab
13e39d6
@terhunej
Formatting, Secrets Manager, Context
1609d5d 
Added additional spacing around images and fixed script image.

Changed setup template to use Secrets Manager instead of a static password.

Added additional context around what commands set up and what the final script does.
@terhunej
warnings
c51b667 
Adding warnings about cost.
@terhunej terhunej requested a review from tebanieo March 16, 2024 07:19
switch180
switch180 reviewed Mar 21, 2024
View reviewed changes
contentspec.yaml
@@ -9,6 +9,7 @@ params:
design_patterns_s3_lab_yaml : "https://s3.amazonaws.com/amazon-dynamodb-labs.com/assets/C9.yaml"
lhol_migration_setup_yaml : "https://s3.amazonaws.com/amazon-dynamodb-labs.com/assets/migration-env-setup.yaml"
lhol_migration_dms_setup_yaml : "https://s3.amazonaws.com/amazon-dynamodb-labs.com/assets/migration-dms-setup.yaml"
lhol_ddb_os_zetl_setup_yaml : "https://s3.amazonaws.com/amazon-dynamodb-labs.com/assets/dynamodb-opensearch-setup.yaml"
Copy link
Contributor

Choose a reason for hiding this comment

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

Pushed to WS repo in commit d74a2a4. This means that this reference is locked in, and needs to be manually updated in the WS contentspec if the path is changed.

switch180
switch180 requested changes Mar 21, 2024
View reviewed changes
Copy link
Contributor

@switch180 switch180 left a comment

Choose a reason for hiding this comment

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

I have reviewed this lab from a deployment perspective. I have prepared this lab for merge, pending approval. It will build the OS pipeline ZIP and copy it to S3 on deploy.

Please review my comment and provide guidance on the origins of the bedrock_query.py script.

Once all changes are complete and this is ready, I will squash merge and then push it to the lab guide in WS.

static/files/hands-on-labs/OpenSearchPipeline/bedrock_query.py Outdated Show resolved Hide resolved
@switch180 switch180 changed the title Opensearch lab LHOL: zETL Opensearch lab Mar 21, 2024
@switch180
Copy link
Contributor

switch180 commented Mar 22, 2024 •
edited
Loading

Testing on WS now...

Edit: Works correctly! No permissions errors, end to end test is good. Just need to address my next comment(s)

@switch180 switch180 self-requested a review March 22, 2024 22:12
@switch180
Copy link
Contributor

switch180 commented Mar 22, 2024 •
edited
Loading

Changes requested. The stack took 26 minutes to launch today. This is untenable in a hands on lab situation due to the delays and extra labor this will take.

  • Move the OS template into WS and add it to the contentspec so it is launched during provisioning. Ensure the exact same copy of the template is the one you publish in this repo - it will be rsync'd from this repo and overwrite the version in WS with each update (done manually by executing a shell script not found in source control)
  • Update the lab guide in line with LHOL/LGME so it clearly explains the at-home versus at-aws-event options. This means you should more or less copy the lab guide sections from one of those labs, being sure that the hyperlinks are correct
  • Resolve code comments (adding those now)

Also, I want to point out that our style guide has guidance that may affect the username that you chose. Please consider changing the username of the OS user, understanding that you will need update all related screenshots as well.

switch180 and others added 5 commits March 22, 2024 18:10
@switch180
Updating w latest template view on stack create
eee6572
@terhunej
Implemented Requested Changes
8235b07 
Added Global Tenant step in OS login.
Added sample Dev Tools query.
Added additional final step queries and item creation.
Fixed curl/unzip in game-player-data lab.
@switch180
Fixed links, ordering
4d13ed1
@switch180
Re-ordered workshops
5ed047b
@switch180
Moving LETL/zETL assets to own folder
75c36c3
@switch180
Copy link
Contributor

Testing in WS. Running into failures unrelated to this lab. Will work on merging tomorrow.

@terhunej
Copy link
Contributor Author

terhunej commented Mar 29, 2024 via email

@switch180 switch180 changed the title LHOL: zETL Opensearch lab LBED: zETL Opensearch lab Mar 29, 2024
@switch180 switch180 merged commit 502022a into aws-samples:master Mar 29, 2024
1 check passed
@switch180 switch180 mentioned this pull request Mar 30, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tebanieo tebanieo tebanieo approved these changes

@switch180 switch180 switch180 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.
4 participants
@terhunej @tebanieo @amdhing @switch180