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.

Sign up for GitHub

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

Jump to bottom

Add option to lock indexes in S3 using DynamoDB & add option to put root index #80

Merged
merged 9 commits into from
May 20, 2021
Merged

Add option to lock indexes in S3 using DynamoDB & add option to put root index #80

merged 9 commits into from
May 20, 2021

Conversation

mdwint
Copy link
Member

@mdwint mdwint commented May 19, 2021 •
edited
Loading

This is a starting point for the changes related to locking discussed in #18.

To do:

  • Add Terraform config for creating the DynamoDB table (should be opt-in).
  • Implement lock and unlock methods.
  • Make AWS profile and region configurable for DynamoDB client.
  • (Optional) Update the root index (this may become a separate PR).
  • Add tests.
  • Update changelog and README.

Fixes #18, closes #26.

@mdwint mdwint mentioned this pull request May 19, 2021
Closed
@mdwint mdwint force-pushed the feature/locking branch 7 times, most recently from ddcc23f to ce71090 Compare May 19, 2021 16:52
@mdwint mdwint force-pushed the feature/locking branch from ce71090 to 807b725 Compare May 19, 2021 17:18
@mdwint mdwint changed the title Locking indexes in S3 using DynamoDB Add option to lock indexes in S3 using DynamoDB May 19, 2021
@mdwint mdwint force-pushed the feature/locking branch from e1d7faf to d1ff074 Compare May 19, 2021 19:01
@mdwint mdwint force-pushed the feature/locking branch from d1ff074 to 06b60f5 Compare May 19, 2021 19:07
@mdwint mdwint force-pushed the feature/locking branch from c0978f7 to 8e6b551 Compare May 19, 2021 20:16
@mdwint mdwint force-pushed the feature/locking branch from 20906c7 to d88e82a Compare May 19, 2021 21:10
@mdwint mdwint force-pushed the feature/locking branch from d88e82a to b2100d5 Compare May 19, 2021 21:24
@mdwint mdwint marked this pull request as draft May 19, 2021 21:26
@mdwint mdwint requested a review from OliverHofkens May 19, 2021 21:27
@mdwint
Copy link
Member Author

mdwint commented May 19, 2021 •
edited
Loading

This is nearly done, @skwashd. Feel free to try it out. I'll still add some tests to cover the DynamoDB locking code.

@mdwint mdwint force-pushed the feature/locking branch from fb87493 to bb27c06 Compare May 19, 2021 23:34
@mdwint mdwint force-pushed the feature/locking branch from a1c0c9a to a21e5f1 Compare May 20, 2021 06:29
@mdwint mdwint changed the title Add option to lock indexes in S3 using DynamoDB Add option to lock indexes in S3 using DynamoDB & add option to put root index May 20, 2021
@mdwint mdwint force-pushed the feature/locking branch from 41ee42f to 4c2e1bb Compare May 20, 2021 07:29
@mdwint mdwint marked this pull request as ready for review May 20, 2021 07:33
OliverHofkens
OliverHofkens approved these changes May 20, 2021
View reviewed changes
Copy link
Member

@OliverHofkens OliverHofkens left a comment

Choose a reason for hiding this comment

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

Looks good!

@mdwint mdwint merged commit 3210194 into master May 20, 2021
@mdwint mdwint deleted the feature/locking branch May 20, 2021 07:54
@nsoranzo nsoranzo mentioned this pull request May 20, 2021
Open
@skwashd
Copy link

skwashd commented May 20, 2021

Thanks for smashing this out so quickly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OliverHofkens OliverHofkens OliverHofkens approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Is there a way to add an Index.html for the entire bucket.
3 participants
@mdwint @skwashd @OliverHofkens