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

feat(Mutations): Idempotent Resume #854

Merged
merged 47 commits into from
Nov 8, 2024
Merged

Conversation

texastony
Copy link
Contributor

@texastony texastony commented Oct 11, 2024

Issue #, if available:

Description of changes:
Refactor Storage:

  • Rename Mutation Lock to Mutation Commitment
  • Introduce Mutation Index to describe what items of a Branch Key have been mutated
  • Add Input field to Mutation Commitment
  • Add Ciphertext field to Mutation Commitment
  • When Mutating an item, always write with an optimistic lock
  • Allow Initialize Mutation to over write a Version, instead of only creating a version
  • When Overwriting a Mutation Index, ensure it has not changed
  • Whenever writing for Mutation, ensure the Mutation Commitment's ENC is as expected (along with original and terminal)

Refactor Storage to contain operations that:

  1. Allow for Atomic Mutations (maybe cut later)
  2. Allow for Deleting a Mutation
  3. Allow for Creating a Mutation Index

Refactor KeyStoreAdmin:

  • Support a System Key for Mutations
  • Stub out the System Key
  • Logic for handling Mutation Index

Refactor Initialize Mutation:

  • If Commitment & Index already exist and match Input, write nothing and return token
  • If Commitment already exists and matches input, write index, and return token
  • If Commitment already exists and does not match input, fail
  • If no commitment, Initialize Mutation

Refactor Apply Mutation:

  • Write an update Page Index.

Squash/merge commit message, if applicable:

feat(Mutations): Idempotent Resume

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@texastony texastony marked this pull request as ready for review October 18, 2024 22:57
@texastony texastony requested a review from a team as a code owner October 18, 2024 22:57
@texastony texastony marked this pull request as draft October 18, 2024 22:57
Copy link

Detected changes to the release files or to the check-files action

Copy link

Changes to the release files or the check-files action requires 2 approvals from CODEOWNERS

Dafny code edition

Co-authored-by: José Corella <[email protected]>
Copy link

github-actions bot commented Nov 7, 2024

@texastony and @josecorella, I noticed you are updating the smithy model files.
Does this update need new or updated user documentation?
Are you adding constraints inside list, map or union? Do you know about this issue: smithy-lang/smithy-dafny#491?

Copy link

github-actions bot commented Nov 7, 2024

@texastony and @josecorella, I noticed you are updating the smithy model files.
Does this update need new or updated user documentation?
Are you adding constraints inside list, map or union? Do you know about this issue: smithy-lang/smithy-dafny#491?

2 similar comments
Copy link

github-actions bot commented Nov 7, 2024

@texastony and @josecorella, I noticed you are updating the smithy model files.
Does this update need new or updated user documentation?
Are you adding constraints inside list, map or union? Do you know about this issue: smithy-lang/smithy-dafny#491?

Copy link

github-actions bot commented Nov 7, 2024

@texastony and @josecorella, I noticed you are updating the smithy model files.
Does this update need new or updated user documentation?
Are you adding constraints inside list, map or union? Do you know about this issue: smithy-lang/smithy-dafny#491?

More Dafny changes

Co-authored-by: José Corella <[email protected]>
Copy link

github-actions bot commented Nov 7, 2024

@texastony and @josecorella, I noticed you are updating the smithy model files.
Does this update need new or updated user documentation?
Are you adding constraints inside list, map or union? Do you know about this issue: smithy-lang/smithy-dafny#491?

Copy link
Contributor

@josecorella josecorella left a comment

Choose a reason for hiding this comment

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

looking good

Copy link

github-actions bot commented Nov 7, 2024

@texastony and @josecorella, I noticed you are updating the smithy model files.
Does this update need new or updated user documentation?
Are you adding constraints inside list, map or union? Do you know about this issue: smithy-lang/smithy-dafny#491?

texastony added a commit that referenced this pull request Nov 7, 2024
Explicitly:
- Change InitializeMutationFlag from a union to an enum for ToString reasons
- Model `DoNotVersion` flag for Initialize Mutation
- Refactor Describe Mutation output to detail Input so resume can be done
- Refactor System Key to be optional, detailing that TrustStorage is the default
- More errors
- Smithy changes from PR feedback on #854
- Correction of spelling mistakes

Why change the flag to an enum?

Dafny/Smithy-Dafny's support for Union's results in structures that do not print well.
The intention of the  `InitializeMutationFlag` is to inform customers
about the result of their request.

Such information may,
possibly even should,
be logged.
Copy link

github-actions bot commented Nov 7, 2024

@texastony and @texastony, I noticed you are updating the smithy model files.
Does this update need new or updated user documentation?
Are you adding constraints inside list, map or union? Do you know about this issue: smithy-lang/smithy-dafny#491?

1 similar comment
Copy link

github-actions bot commented Nov 7, 2024

@texastony and @texastony, I noticed you are updating the smithy model files.
Does this update need new or updated user documentation?
Are you adding constraints inside list, map or union? Do you know about this issue: smithy-lang/smithy-dafny#491?

texastony added a commit that referenced this pull request Nov 8, 2024
Initialize Mutation and Apply Mutation MUST
ensure that the UUID of the Index and Commitment agree.
Apply MUST ensure that the UUID of the Commitment and Token agree.

Refactored some of the error messages.
Utilize Java Example to demonstrate resume and restart.
Finally, addressed some of the feedback on PR #854.
This reverts commit 4b54cbc.
Instead, these suggestions were consumed in PR #955.
This reverts commit 620f96e.
Instead, these suggestions were consumed in PR #955.
texastony added a commit that referenced this pull request Nov 8, 2024
Explicitly:
- Change InitializeMutationFlag from a union to an enum for ToString reasons
- Model `DoNotVersion` flag for Initialize Mutation
- Refactor Describe Mutation output to detail Input so resume can be done
- Refactor System Key to be optional, detailing that TrustStorage is the default
- More errors
- Smithy changes from PR feedback on #854
- Correction of spelling mistakes
- Mutation Token's UUID is required

Why change the flag to an enum?

Dafny/Smithy-Dafny's support for Union's results in structures that do not print well.
The intention of the  `InitializeMutationFlag` is to inform customers
about the result of their request.

Such information may,
possibly even should,
be logged.
texastony added a commit that referenced this pull request Nov 8, 2024
Initialize Mutation and Apply Mutation MUST
ensure that the UUID of the Index and Commitment agree.
Apply MUST ensure that the UUID of the Commitment and Token agree.
The Mutation Token's UUID is REQUIRED.
It is how we track a mutation,
much like how CFN tracks a change set.

Refactored some of the error messages.
Utilize Java Example to demonstrate resume and restart.
Finally, addressed some of the feedback on PR #854.
texastony added a commit that referenced this pull request Nov 8, 2024
Initialize Mutation and Apply Mutation MUST
ensure that the UUID of the Index and Commitment agree.
Apply MUST ensure that the UUID of the Commitment and Token agree.
The Mutation Token's UUID is REQUIRED.
It is how we track a mutation,
much like how CFN tracks a change set.

Fixed bug where UUID is a reserved word in DDB.

Refactored some of the error messages.
Utilize Java Example to demonstrate resume and restart.
Finally, addressed some of the feedback on PR #854.
texastony added a commit that referenced this pull request Nov 8, 2024
Initialize Mutation and Apply Mutation MUST
ensure that the UUID of the Index and Commitment agree.
Apply MUST ensure that the UUID of the Commitment and Token agree.
The Mutation Token's UUID is REQUIRED.
It is how we track a mutation,
much like how CFN tracks a change set.

Fixed bug where UUID is a reserved word in DDB.

Refactored some of the error messages.
Utilize Java Example to demonstrate resume and restart.
Finally, addressed some of the feedback on PR #854.
@texastony texastony merged commit f6b42b9 into rc-1.8.0 Nov 8, 2024
95 checks passed
@texastony texastony deleted the tony/mutations-idempotent branch November 8, 2024 03:19
texastony added a commit that referenced this pull request Nov 8, 2024
Explicitly:
- Change InitializeMutationFlag from a union to an enum for ToString reasons
- Model `DoNotVersion` flag for Initialize Mutation
- Refactor Describe Mutation output to detail Input so resume can be done
- Refactor System Key to be optional, detailing that TrustStorage is the default
- More errors
- Smithy changes from PR feedback on #854
- Correction of spelling mistakes
- Mutation Token's UUID is required

Why change the flag to an enum?

Dafny/Smithy-Dafny's support for Union's results in structures that do not print well.
The intention of the  `InitializeMutationFlag` is to inform customers
about the result of their request.

Such information may,
possibly even should,
be logged.

Initialize Mutation and Apply Mutation MUST
ensure that the UUID of the Index and Commitment agree.
Apply MUST ensure that the UUID of the Commitment and Token agree.
The Mutation Token's UUID is REQUIRED.
It is how we track a mutation,
much like how CFN tracks a change set.

Fixed bug where UUID is a reserved word in DDB.

Refactored some of the error messages.
Utilize Java Example to demonstrate resume and restart.
Finally, addressed some of the feedback on PR #854.
texastony added a commit that referenced this pull request Nov 8, 2024
Explicitly:
- Change InitializeMutationFlag from a union to an enum for ToString reasons
- Model `DoNotVersion` flag for Initialize Mutation
- Refactor Describe Mutation output to detail Input so resume can be done
- Refactor System Key to be optional, detailing that TrustStorage is the default
- More errors
- Smithy changes from PR feedback on #854
- Correction of spelling mistakes
- Mutation Token's UUID is required

Why change the flag to an enum?

Dafny/Smithy-Dafny's support for Union's results in structures that do not print well.
The intention of the  `InitializeMutationFlag` is to inform customers
about the result of their request.

Such information may,
possibly even should,
be logged.

Initialize Mutation and Apply Mutation MUST
ensure that the UUID of the Index and Commitment agree.
Apply MUST ensure that the UUID of the Commitment and Token agree.
The Mutation Token's UUID is REQUIRED.
It is how we track a mutation,
much like how CFN tracks a change set.

Fixed bug where UUID is a reserved word in DDB.

Refactored some of the error messages.
Utilize Java Example to demonstrate resume and restart.
Finally, addressed some of the feedback on PR #854.
texastony added a commit that referenced this pull request Nov 11, 2024
Explicitly:
- Change InitializeMutationFlag from a union to an enum for ToString reasons
- Model `DoNotVersion` flag for Initialize Mutation
- Refactor Describe Mutation output to detail Input so resume can be done
- Refactor System Key to be optional, detailing that TrustStorage is the default
- More errors
- Smithy changes from PR feedback on #854
- Correction of spelling mistakes
- Mutation Token's UUID is required

Why change the flag to an enum?

Dafny/Smithy-Dafny's support for Union's results in structures that do not print well.
The intention of the  `InitializeMutationFlag` is to inform customers
about the result of their request.

Such information may,
possibly even should,
be logged.

Initialize Mutation and Apply Mutation MUST
ensure that the UUID of the Index and Commitment agree.
Apply MUST ensure that the UUID of the Commitment and Token agree.
The Mutation Token's UUID is REQUIRED.
It is how we track a mutation,
much like how CFN tracks a change set.

Fixed bug where UUID is a reserved word in DDB.

Refactored some of the error messages.
Utilize Java Example to demonstrate resume and restart.
Finally, addressed some of the feedback on PR #854.
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