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

Added localstack batch and locale2e #2001

Merged
merged 2 commits into from
Jul 19, 2021
Merged

Added localstack batch and locale2e #2001

merged 2 commits into from
Jul 19, 2021

Conversation

jim-sheldon
Copy link
Collaborator

  • Create S3 buckets
  • Create Batch Job Queue
  • Add email address to SES
  • Curator AWS clients use localstack when using locale2e docker-compose stack
  • Curator async download uses localstack when using locale2e docker-compose stack

@jim-sheldon jim-sheldon force-pushed the 1932-local-e2e-env branch 3 times, most recently from 5cc8c05 to 964825e Compare July 14, 2021 23:28
@codecov-commenter
Copy link

codecov-commenter commented Jul 14, 2021

Codecov Report

Merging #2001 (aa35ca8) into main (999e0a1) will decrease coverage by 19.12%.
The diff coverage is 71.42%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2001       +/-   ##
===========================================
- Coverage   81.40%   62.28%   -19.13%     
===========================================
  Files          56       96       +40     
  Lines        1845     3516     +1671     
  Branches      337      987      +650     
===========================================
+ Hits         1502     2190      +688     
- Misses        343     1326      +983     
Impacted Files Coverage Δ
verification/curator-service/api/src/index.ts 88.80% <ø> (ø)
...ation/curator-service/api/src/util/validate-env.ts 100.00% <ø> (ø)
...tion/curator-service/ui/src/components/App/App.tsx 79.60% <ø> (ø)
...rator-service/api/src/clients/aws-lambda-client.ts 62.50% <60.00%> (-1.14%) ⬇️
...urator-service/api/src/clients/aws-batch-client.ts 95.65% <75.00%> (-2.03%) ⬇️
...rator-service/api/src/clients/aws-events-client.ts 96.96% <80.00%> (-3.04%) ⬇️
...g/data-service/src/model/preexisting-conditions.ts
data-serving/data-service/src/controllers/home.ts
data-serving/data-service/src/util/case.ts
data-serving/data-service/src/model/date-range.ts
... and 101 more

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 4221aaf...aa35ca8. Read the comment docs.

counter += 1
print("Waiting for localstack")
sleep(5)
raise Exception("Localstack not available")
Copy link
Contributor

Choose a reason for hiding this comment

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

When I try running dev/run_full_stack.sh locally I hit this exception, is there any setup I need to do? I've set LOCALSTACK_API_KEY in dev/.env.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I suspect a timing issue. I increased the number of attempts to 20.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can't reproduce this, but I got this error message:

Error response from daemon: invalid mount config for type "bind": bind source path does not exist: /tmp/localstack

Creating /tmp/localstack fixed this

Copy link
Contributor

Choose a reason for hiding this comment

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

Creating /tmp/localstack fixed this

Should that ideally go in the host's /tmp or the docker image's /tmp? Either way, it needs to be scripted.

Copy link
Collaborator Author

@jim-sheldon jim-sheldon Jul 15, 2021

Choose a reason for hiding this comment

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

Strange, I didn't run into that. FWIW /tmp exists in the docker image, but I'm not using it for anything--the volume declared in the compose file was a default--so I removed it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I still see that the timeout exception is raised before the localstack has launched:

setup-localstack_1  | Waiting for localstack
setup-localstack_1  | Traceback (most recent call last):
setup-localstack_1  |   File "./setup_localstack.py", line 177, in <module>
setup-localstack_1  |     wait_for_localstack()
setup-localstack_1  |   File "./setup_localstack.py", line 172, in wait_for_localstack
setup-localstack_1  |     raise Exception("Localstack not available")
setup-localstack_1  | Exception: Localstack not available
setup-localstack_1 exited with code 1
localstack_1        | Waiting for all LocalStack services to be ready
localstack_1        | 2021-07-16T08:25:19:INFO:botocore.credentials: Found credentials in environment variables.
localstack_1        | Waiting for all LocalStack services to be ready
localstack_1        | Ready.
localstack_1        | 2021-07-16 08:25:24,808:API: 127.0.0.1 - - [16/Jul/2021 08:25:24] "GET / HTTP/1.1" 200 -
localstack_1        | 2021-07-16T08:25:24:INFO:localstack.utils.analytics.profiler: Execution of "start_api_services" took 12558.80ms

Could the setup image use wait-for-it.sh and run when the localstack service is finally accepting connections?

Copy link
Contributor

@abhidg abhidg left a comment

Choose a reason for hiding this comment

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

Couple of small changes, looks good otherwise!

dev/run_full_stack.sh Outdated Show resolved Hide resolved
dev/setup_localstack.py Outdated Show resolved Hide resolved
@jim-sheldon jim-sheldon force-pushed the 1932-local-e2e-env branch 3 times, most recently from 88f9ebf to 5cf4f93 Compare July 15, 2021 15:29
@jim-sheldon jim-sheldon changed the title Added localstack and locale2e Added localstack batch and locale2e Jul 15, 2021
@abhidg abhidg self-requested a review July 16, 2021 13:34
@iamleeg
Copy link
Contributor

iamleeg commented Jul 19, 2021

I see the same test failure in FF on a different PR. I'll try to fix and get a commit for you to cherry-pick.

@iamleeg
Copy link
Contributor

iamleeg commented Jul 19, 2021

I'm not proud but df56a02 works.

of pausing after clicking the button before checking for the menu item
@iamleeg iamleeg self-requested a review July 19, 2021 13:21
@jim-sheldon jim-sheldon merged commit 4b5e448 into main Jul 19, 2021
@jim-sheldon jim-sheldon deleted the 1932-local-e2e-env branch July 19, 2021 13:35
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.

4 participants