stac-asset icon indicating copy to clipboard operation
stac-asset copied to clipboard

Warn (not error) on content type, by default

Open gadomski opened this issue 1 year ago • 1 comments

Related issues and pull requests

  • Closes #223

Description

@scottyhq any chance you have time to check this one out? I don't have a content_type-non-complaint server at the ready.

gadomski avatar Oct 09 '24 01:10 gadomski

Codecov Report

Attention: Patch coverage is 46.15385% with 7 lines in your changes missing coverage. Please review.

Project coverage is 83.79%. Comparing base (40b6c71) to head (9c92c6b).

Files with missing lines Patch % Lines
src/stac_asset/http_client.py 36.36% 7 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #227      +/-   ##
==========================================
- Coverage   84.15%   83.79%   -0.37%     
==========================================
  Files          16       16              
  Lines        1073     1080       +7     
==========================================
+ Hits          903      905       +2     
- Misses        170      175       +5     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Oct 09 '24 01:10 codecov-commenter

@scottyhq any chance you have time to check this one out?

Thanks for this! I'll be able to test it out later next week

scottyhq avatar Oct 11 '24 07:10 scottyhq

👍 for this change! I recently encountered the issue where content_type only returns the content part of the Content-Type header, leading to content type validation fail.

Here's an example in test.py:

import asyncio
from aiohttp import ClientSession

async def fetch_image():
    url = "https://..."
    async with ClientSession() as session:
        async with session.get(url) as response:
            print(response.content_type)
            print(response.headers['Content-Type'])

# Run the async function using asyncio
if __name__ == "__main__":
    asyncio.run(fetch_image())

Output:

$ python test.py
image/tiff
image/tiff; application=geotiff

drnextgis avatar Oct 23 '24 07:10 drnextgis

@drnextgis thanks! I tried to be permissive about geotiff content types even before this PR, but I forgot about non-cloud-optimized GeoTIFF. I'll add this to PR ... any chance you could check it out, or point me to a server that has this problem so I can test it myself. I haven't dug around to find out yet.

gadomski avatar Oct 23 '24 11:10 gadomski

Thank you! I can confirm that the warning is triggered when the content type check fails (tested via CLI):

0/8: : 0.00B [00:00, ?B/s, 0 errors]/home/denis/git/stac-asset/src/stac_asset/http_client.py:175: UserWarning: the actual content type does not match the expected: actual=image/tiff1, expected=image/tiff; application=geotiff
  warnings.warn(str(err))

I can also confirm that it no longer fails with image/tiff; application=geotiff.

drnextgis avatar Oct 23 '24 12:10 drnextgis

Released: https://github.com/stac-utils/stac-asset/releases/tag/v0.4.4

gadomski avatar Oct 23 '24 12:10 gadomski