-
Notifications
You must be signed in to change notification settings - Fork 289
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
🪲 Running programs throws an error if logged in #5513
Conversation
When logged in, we automatically save a program as the user runs it. In a recent change, we update the program counter only when a program is modified from the example code. But we wrote the entire 'program' record back to the database in an `update()` call, which includes the `id` field; Dynamo does not allow including the `id` field in an `update()` call. Instead, only update the `is_modified` field, and update the Dynamo abstraction layer to check for this error condition.
for more information, see https://pre-commit.ci
@Annelein this should have thrown an error locally before deploying and running against an actual DynamoDB, so my apologies that it didn't. But also,
|
for more information, see https://pre-commit.ci
Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork). |
Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork). |
@rix0rrr thank you for the feedback, fix and explanation! Really helpful. |
When logged in, we automatically save a program as the user runs it.
In a recent change, we update the program counter only when a program is modified from the example code. But we wrote the entire 'program' record back to the database in an
update()
call, which includes theid
field; Dynamo does not allow including theid
field in anupdate()
call.This is also not necessary: a DynamoDB
update()
call only needs to include the fields we actually want to change, and not any of the fields that are unchanged (this may even lead to a race condition if any of those fields were changed in between the read and update calls, we would undo the intermediate change again).Instead, only update the
is_modified
field, and update the Dynamo abstraction layer to check for this error condition.How to test
Log in, and run a program.