Skip to content

remove S3 persistence workaround #11809

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

Merged
merged 2 commits into from
Nov 13, 2024
Merged

remove S3 persistence workaround #11809

merged 2 commits into from
Nov 13, 2024

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Nov 7, 2024

Motivation

When implementing new features in S3 since 3.0.0, we've taken extra care to not break persistence by using getattr around store attributes and have additional conditionals in place.

As we're approaching a major release, I've collected the TODO related to this and removed the getattr blocks to simplify the logic. If people loaded earlier state and then saved it again after using it with a version with those calls, the state should not be broken anyway.

Changes

  • remove TODOs and getattr around store operations, as v4 is probably going to break persistence in multiple ways with all the provider changes.

@bentsku bentsku added aws:s3 Amazon Simple Storage Service semver: major Breaking changes which can be included in a major release only labels Nov 7, 2024
@bentsku bentsku added this to the 4.0 milestone Nov 7, 2024
@bentsku bentsku self-assigned this Nov 7, 2024
@bentsku bentsku mentioned this pull request Nov 7, 2024
9 tasks
Copy link

github-actions bot commented Nov 7, 2024

S3 Image Test Results (AMD64 / ARM64)

  2 files  ±0    2 suites  ±0   5m 20s ⏱️ + 1m 28s
421 tests ±0  369 ✅ ±0   52 💤 ±0  0 ❌ ±0 
842 runs  ±0  738 ✅ ±0  104 💤 ±0  0 ❌ ±0 

Results for commit 10e46e0. ± Comparison against base commit 39769d1.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Nov 7, 2024

LocalStack Community integration with Pro

    2 files      2 suites   56m 55s ⏱️
1 571 tests 1 401 ✅ 170 💤 0 ❌
1 573 runs  1 401 ✅ 172 💤 0 ❌

Results for commit 10e46e0.

♻️ This comment has been updated with latest results.

@bentsku bentsku force-pushed the s3-state-check branch 2 times, most recently from 5fe34da to f12f6a3 Compare November 8, 2024 17:53
@bentsku bentsku marked this pull request as ready for review November 8, 2024 21:40
@alexrashed alexrashed removed their request for review November 10, 2024 15:44
Copy link
Contributor

@cloutierMat cloutierMat left a comment

Choose a reason for hiding this comment

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

LGTM!

The power of TODOs in display here

@bentsku bentsku merged commit 29b0b8c into release/v4 Nov 13, 2024
36 checks passed
@bentsku bentsku deleted the s3-state-check branch November 13, 2024 11:06
dfangl pushed a commit that referenced this pull request Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws:s3 Amazon Simple Storage Service semver: major Breaking changes which can be included in a major release only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants