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

Test and develop against Dynamodb Local #179

Merged
merged 9 commits into from
Dec 19, 2024

Conversation

rustworthy
Copy link
Contributor

This allow for running e2e tests and developing against a local copy of DynamoDB with the ability to view changes in the admin panel:

image

server/src/main.rs Outdated Show resolved Hide resolved
server/Makefile Outdated Show resolved Hide resolved
@jonhoo
Copy link
Owner

jonhoo commented Nov 25, 2024

This is great, thank you!

@rustworthy
Copy link
Contributor Author

This is great, thank you!

It's my pleasure actually.

I am wondering if the issues section is up-to-date, or some feature requests/issues are missing.

@rustworthy rustworthy requested a review from jonhoo November 26, 2024 07:25
Copy link
Owner

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

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

The issues list should be up to date! At least I don't recall any other features I've wanted to be added since.

One thought: now that we have local dynamo, maybe we could make CI run the tests as well? Ideally one day we also have tests of the frontend, but at least running cargo test would be a start 🎉

server/Makefile Outdated Show resolved Hide resolved
server/run-dynamodb-local.sh Outdated Show resolved Hide resolved
@jonhoo
Copy link
Owner

jonhoo commented Dec 15, 2024

Oh, I suppose there is also bringing #10 over the line

@jonhoo
Copy link
Owner

jonhoo commented Dec 19, 2024

Thank you!

Comment on lines +49 to +52
AWS_DEFAULT_REGION=us-east-1 \
AWS_ACCESS_KEY_ID=lorem \
AWS_SECRET_ACCESS_KEY=ipsum \
AWS_ENDPOINT_URL=http://localhost:8000 \
Copy link
Owner

Choose a reason for hiding this comment

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

Should we set it up so that there's just one environment variable to set (like USE_DYNAMODB=test), and when it's set like that then we set the others accordingly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this setup is possible: we will need to check the USE_DYNAMODB variable and, if it is set to test, construct the config accordingly using the builder. Let me look into into it.

@jonhoo jonhoo merged commit 25a0baf into jonhoo:main Dec 19, 2024
3 checks passed
@rustworthy rustworthy deleted the dynamodb-local branch December 26, 2024 12:40
jonhoo added a commit that referenced this pull request Dec 27, 2024
PR #179 broke release builds by making it necessary for
`LiveAskQuestion` and `SEED` to both now be available regardless of
whether `debug_assertions` is set. That's unfortunate, since `SEED` in
particular would make the binary quite large. So I've instead made it so
that in release mode (or rather, without `debug_assertions`), `SEED` is
_not_ compiled in, and DynamoDB mode is _always_ used.

I've also added a CI job to check `--release --target
aarch64-unknown-linux-gnu` so that this can't happen in the future.
@jonhoo jonhoo mentioned this pull request Dec 27, 2024
jonhoo added a commit that referenced this pull request Dec 27, 2024
PR #179 broke release builds by making it necessary for
`LiveAskQuestion` and `SEED` to both now be available regardless of
whether `debug_assertions` is set. That's unfortunate, since `SEED` in
particular would make the binary quite large. So I've instead made it so
that in release mode (or rather, without `debug_assertions`), `SEED` is
_not_ compiled in, and DynamoDB mode is _always_ used.

I've also added a CI job to check `--release --target
aarch64-unknown-linux-gnu` so that this can't happen in the future.
This was referenced Dec 28, 2024
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.

2 participants