Require LocalStack license and standardize sample documentation#269
Require LocalStack license and standardize sample documentation#269HarshCasper wants to merge 5 commits intomasterfrom
Conversation
…amples - Add LOCALSTACK_AUTH_TOKEN check to all `start` Makefile targets - Remove deprecated ACTIVATE_PRO=1 from all Makefiles - Add `check` target for prerequisite validation to all Makefiles - Create Makefiles for 11 samples that were missing them - Rewrite all sample READMEs to match gold standard format with key/value tables - Add license prerequisite as first bullet in all READMEs - Create missing READMEs for cdk-for-terraform, emr-serverless-sample, lambda-function-urls-python, lambda-hot-reloading, lambda-xray - Update root README Configuration section and add 11 missing table entries - Fix broken lambda-function-urls link in root README
0d984cc to
a2c239a
Compare
- Update all 48 Makefiles: change error URL from /pricing to /workspace/auth-token so users land on the token page directly - Update all 48 READMEs: expand license bullet to include explicit export LOCALSTACK_AUTH_TOKEN=<your-auth-token> instruction with a link to the LocalStack Web Application - Fix apigw-custom-domain/README.md: add missing license bullet, Docker, localstack CLI, and awslocal CLI prerequisites; remove stale Environment row from the metadata table
- Revert Prerequisites license bullet to simple one-liner (no inline code block or web app link) - Add explicit 'export LOCALSTACK_AUTH_TOKEN=<your-auth-token>' as the first command in the Start LocalStack code block across all sample READMEs (47 make start + 1 localstack start variant)
remotesynth
left a comment
There was a problem hiding this comment.
We seem to have found some of the limitations to using AI here as it seems to have made some major changes in trying to enforce a rigid format on all of these readme's and make files. In some cases, I worry the changes remove necessary context, additional instructions or expected outputs and in some others I am not sure it changed things in a way that will actually work.
| The demo will run different GraphQL queries, for two different datasources (DynamoDB / RDS Aurora): | ||
| ## Run the application | ||
|
|
||
| 1. a mutation query which inserts a new item into DynamoDB / RDS Aurora |
There was a problem hiding this comment.
Do we actually want to remove the expected output here? It seems that here and below all the expected results have just been replaced by make commands and a few bullet points.
| make run | ||
| ``` | ||
|
|
||
| The demo script performs the following actions: |
There was a problem hiding this comment.
Here again it seems to have aggressively removed details on what will happen and expected outputs, which lessens the learning value for the developer reviewing this sample.
|
|
||
| ```bash | ||
| make deploy | ||
| ``` |
There was a problem hiding this comment.
In this case, I think the lack of detail is ok for now because it appears there was no readme at all previously, but in the others above there was but useful detail was removed.
|
|
||
| Make sure that LocalStack is started: | ||
| ``` | ||
| LOCALSTACK_AUTH_TOKEN=... DEBUG=1 localstack start |
There was a problem hiding this comment.
Not sure if it's important or not, but this instruction being removed says to use debug, but the make file start does not use it.
| make start | ||
| ``` | ||
|
|
||
| docker run --rm -it -p 3000:80 -p 2525:25 rnwood/smtp4dev |
There was a problem hiding this comment.
These instructions seem substantively different than what's in the make file.
| ``` | ||
| ```bash | ||
| export LOCALSTACK_AUTH_TOKEN=<your-auth-token> | ||
| make start |
| make start | ||
| ``` | ||
|
|
||
| You may need to confirm the creation by entering "yes". You should then see log output similar to the one below: |
There was a problem hiding this comment.
All of this context and output seem necessary and useful
| ## Run Example | ||
| * Import the project (e.g. in IntelliJ), | ||
| * configure your LOCALSTACK_AUTH_TOKEN as environment variable, | ||
| * and then run the test `TestRDS`. |
There was a problem hiding this comment.
Based on what I can tell, this may not be what the make file is doing.
| LOCALSTACK_AUTH_TOKEN=... DEBUG=1 localstack start | ||
| ```bash | ||
| export LOCALSTACK_AUTH_TOKEN=<your-auth-token> | ||
| make start |
| make run | ||
| ``` | ||
|
|
||
| You should then see a couple of log messages in the terminal: |
Fixes DEVREL-2
Summary
LOCALSTACK_AUTH_TOKENcheck to allstartMakefile targets so users are clearly told to set their auth token before starting LocalStackACTIVATE_PRO=1env var from all Makefilescheckprerequisite validation target to all Makefilescdk-for-terraform,cdk-resources,chalice-rest-api,cloudwatch-metrics-aws,emr-serverless-sample,emr-serverless-spark,java-notification-app,lambda-event-filtering,lambda-hot-reloading,sagemaker-inference,testcontainers-java-samplecdk-for-terraform,emr-serverless-sample,lambda-function-urls-python,lambda-hot-reloading(top-level),lambda-xray(top-level)lambda-function-urlslinkTest plan
make startfails with a clear error message whenLOCALSTACK_AUTH_TOKENis not setmake checkcatches missing prerequisitesmake deployandmake runwork as expected for a sample of the updated samples