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

Separate S3Control methods from S3 #4745

Merged
merged 3 commits into from
Jan 18, 2022

Conversation

bblommers
Copy link
Collaborator

The S3-module contained three methods that were technically part of S3Control:

  • delete_public_access_block()
  • get_public_access_block()
  • put_public_access_block

Because they were part of the S3-module, it needed the S3-mock:

@mock_s3
def test():
    client = boto3.client("s3control")
    ...

This PR extracts the logic into a dedicated module, making it more discoverable and obvious that we support these methods.
This means users now have to use the dedicated mock_s3control:

@mock_s3control
def test():
    client = boto3.client("s3control")
    ...

This is a breaking change, so it will only be merged as part of the next major release.

Note that none of the existing logic or tests have been changed as part of this PR - only moved around.

@bblommers bblommers added this to the 3.0.0 milestone Jan 8, 2022
@bblommers
Copy link
Collaborator Author

@mikegrima I believe you implemented this feature originally - any objections to this change from your side?

@mikegrima
Copy link
Collaborator

Hi @bblommers! As long as all of the previous functionality will work properly then I'm OK with the change. Is the breaking change that instead of mock_s3, its mock_s3control?

@bblommers
Copy link
Collaborator Author

Correct @mikegrima - it's just a matter of using the new decorator from 3.x. None of the logic has changed.

@mikegrima
Copy link
Collaborator

LGTM

@bblommers
Copy link
Collaborator Author

Thanks @mikegrima!

@codecov-commenter
Copy link

Codecov Report

Merging #4745 (77d2ad5) into master (9c8744f) will increase coverage by 0.00%.
The diff coverage is 97.20%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4745   +/-   ##
=======================================
  Coverage   95.01%   95.02%           
=======================================
  Files         575      580    +5     
  Lines       61324    61343   +19     
=======================================
+ Hits        58268    58289   +21     
+ Misses       3056     3054    -2     
Flag Coverage Δ
servertests 36.29% <25.87%> (+0.01%) ⬆️
unittests 94.95% <97.20%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
moto/backend_index.py 100.00% <ø> (ø)
moto/s3/config.py 100.00% <ø> (+3.66%) ⬆️
moto/s3/models.py 95.22% <ø> (-0.08%) ⬇️
moto/s3/urls.py 100.00% <ø> (ø)
moto/s3control/config.py 93.44% <93.44%> (ø)
moto/__init__.py 98.42% <100.00%> (+0.01%) ⬆️
moto/config/models.py 99.34% <100.00%> (+<0.01%) ⬆️
moto/s3/responses.py 95.52% <100.00%> (-0.01%) ⬇️
moto/s3control/__init__.py 100.00% <100.00%> (ø)
moto/s3control/models.py 100.00% <100.00%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c8744f...77d2ad5. Read the comment docs.

@bblommers bblommers merged commit 6610862 into getmoto:master Jan 18, 2022
@bblommers bblommers deleted the techdebt/separate-s3-s3control branch January 18, 2022 20:10
@github-actions
Copy link
Contributor

This is now part of moto >= 3.0.0.dev5

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.

None yet

3 participants