-
Notifications
You must be signed in to change notification settings - Fork 11
Restructure project to use pyproject.toml; add secretsmanager proxy test #106
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
Conversation
02fbfd3 to
a36c2b0
Compare
a36c2b0 to
e94c0d4
Compare
e94c0d4 to
f0f5daa
Compare
1de8ba6 to
492168e
Compare
492168e to
da49f57
Compare
| ): | ||
| for bucket in buckets_aws + buckets_proxied: | ||
| # TODO: tmp fix - seems like this attribute is missing for certain boto3 versions. To be investigated! | ||
| bucket.pop("BucketArn", None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: this is a pretty ugly workaround :/ - I didn't have the time to investigate in-depth, but it seems that installing different boto3 versions can result in different attributes being returned here (sometimes BucketArn is present, sometimes missing). Afaict, the S3 tests would currently also fail against latest main branch. Again, not a great solution - but I'd prefer to merge the PR as-is, and then investigate and properly fix this issue separately in a follow-up.. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: Just re-ran the builds on main, and confirmed that the tests are also currently failing on main: https://github.com/localstack/localstack-extensions/actions/runs/18838765454/job/53745841266
buckets_proxied = s3_client.list_buckets()["Buckets"]
buckets_aws = s3_client_aws.list_buckets()["Buckets"]
> assert buckets_proxied and buckets_proxied == buckets_aws
E AssertionError: assert ([{'BucketArn': 'arn:aws:s3:::test-bucket-c9996004', 'CreationDate': datetime.datetime(2025, 10, 27, 11, 10, 4, tzinfo=tzlocal()), 'Name': 'test-bucket-c9996004'}] and [{'BucketArn'...et-c9996004'}] == [{'CreationDa...et-c9996004'}]
E
E At index 0 diff: {'Name': 'test-bucket-c9996004', 'CreationDate': datetime.datetime(2025, 10, 27, 11, 10, 4, tzinfo=tzlocal()), 'BucketArn': 'arn:aws:s3:::test-bucket-c9996004'} != {'Name': 'test-bucket-c9996004', 'CreationDate': datetime.datetime(2025, 10, 27, 11, 10, 4, tzinfo=tzlocal())}
E
E Full diff:
E [
E {
E + 'BucketArn': 'arn:aws:s3:::test-bucket-c9996004',
E 'CreationDate': datetime.datetime(2025, 10, 27, 11, 10, 4, tzinfo=tzlocal()),
E 'Name': 'test-bucket-c9996004',
E },
E ])
tests/test_proxy_requests.py:92: AssertionError
(again, something to be investigated in a follow-up iteration..)
nik-localstack
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (apart from a minor change)
Thanks for the improvements 🙌
I agree with tackling the test failure in a separate step
Co-authored-by: Nikos <nikos.michas@localstack.cloud>
This PR adds a couple of refactorings, to prepare the aws-proxy project for upcoming changes and additional testing:
pyproject.toml, as well as modern build tools (ruffinstead ofblack/flake8, etc)test_proxy_requests.pymodule is starting to become too largeruff(which causes lots of changes in this PR)Mostly refactoring changes in this PR, no functional changes per se - apologies in advance for the large size of the changeset! 🙌