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

support common_prefixes response object parsing. #31

Merged
merged 2 commits into from
Apr 5, 2024

Conversation

toppk
Copy link
Contributor

@toppk toppk commented Mar 31, 2024

resolves #30

resolves aiohttp-s3-client/issues/30
@coveralls
Copy link

coveralls commented Mar 31, 2024

Pull Request Test Coverage Report for Build 8511952455

Details

  • 4 of 7 (57.14%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 90.504%

Changes Missing Coverage Covered Lines Changed/Added Lines %
aiohttp_s3_client/client.py 2 5 40.0%
Totals Coverage Status
Change from base Build 7486418958: 0.02%
Covered Lines: 467
Relevant Lines: 516

💛 - Coveralls

@toppk
Copy link
Contributor Author

toppk commented Apr 1, 2024

i figured out how to run tests (there should be developer docs):

podman run -p 9090:9090 --env initialBuckets=test   docker://adobe/s3mock
S3_URL=http://user:hackme@localhost:9090/test/ poetry run pytest -vv tests

but there is an issue:
if you run:

$ curl 'http://user:hackme@localhost:9090/test/?list-type=2&prefix=test/&delimiter=/&encoding-type=url'
<ListBucketResult><Name>test</Name><Prefix>test/</Prefix><MaxKeys>1000</MaxKeys><IsTruncated>false</IsTruncated><KeyCount>0</KeyCount><EncodingType>url</EncodingType></ListBucketResult>You have new mail in /var/spool/mail/toppk

you see there isn't the xml header with the namespace.

the list objects tests are false negative because there is no response. if you added a bogus assert 1==2 test inside the with block you will see that.

a fix is to set the NS = "" (in parse_list_objects, if you do it a the module level in xml.py, it will break other things).

obviously this is an issue in my s3mock, but given there's a potential for false negative in the way the tests are structured, it may be an issue on your side as well. I've tried to look at s3mock to see if I can figure out what's happening there but nothing so far.

@toppk
Copy link
Contributor Author

toppk commented Apr 1, 2024

i've tested a bunch of older version of s3mock, and same issues.

for references this is what aws s3 gives, which is why this code works in prod (for me).

<?xml version="1.0" encoding="UTF-8"?>
<ListBucketResult xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
...
</ListBucketResult>

@mosquito
Copy link
Collaborator

mosquito commented Apr 2, 2024

@toppk may be s3mock doesn’t support that features? Could you please test it again with minio?

@toppk
Copy link
Contributor Author

toppk commented Apr 3, 2024

minio behaves properly.

$ curl http://MY-MINIO-SERVER/test
<?xml version="1.0" encoding="UTF-8"?>
<ListBucketResult xmlns="http://s3.amazonaws.com/doc/2006-03-01/"><Name>test</Name><Prefix></Prefix><Marker></Marker><MaxKeys>1000</MaxKeys><IsTruncated>false</IsTruncated></ListBucketResult>

I'm not sure if there's some sort of enviriomental/pebcak issue, but I've created an issue on s3mock to see if they can help me out.

adobe/S3Mock#1754

@mosquito mosquito merged commit dde8d46 into aiokitchen:master Apr 5, 2024
8 checks passed
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.

should handle CommonPrefixes in ListObjectsV2
4 participants