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

update titiler-pgstac version and forward Request in reader #91

Merged
merged 6 commits into from
Jun 21, 2022

Conversation

vincentsarago
Copy link
Contributor

This PR does:

  • update titiler-pgstac/titiler version
  • add Request in readers attribute and use titiler reader_dependency to inject the Request object in the reader at request time

pctiler/setup.py Outdated Show resolved Hide resolved
@@ -49,6 +67,10 @@ class MosaicSTACReader(pgstac_mosaic.CustomSTACReader):

reader: Type[BaseReader] = attr.ib(default=CustomCOGReader)

# We make request an optional attribute to avoid re-writing
# the whole list of attribute
request: Optional[Request] = attr.ib(default=None)
Copy link
Member

Choose a reason for hiding this comment

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

With request now optionally on MosaicSTACReader, I expected to be able to access self.reader.request within methods of PGSTACBackend (like assets_for_tiles), but I get the error

AttributeError: type object 'MosaicSTACReader' has no attribute 'request'

The type annotations suggest this is possible, is that the expected usage? I can confirm that ReaderParams was initialized with a request correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can you share some code, I have hard to to follow 😓

Copy link
Member

Choose a reason for hiding this comment

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

Sure, sorry! One use case for having the request available is to use it for our logging within backend/reader methods, e.g., grabbing a trace id from the header so all logs can be associated with a single request. Usage would be like this: f461009, but it doesn't seem like request is available from there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

latest commit should add self.request (in the Backend)

Copy link
Member

@mmcfarland mmcfarland left a comment

Choose a reason for hiding this comment

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

Works great - I can access the request in both the reader and the backend 👍

@mmcfarland mmcfarland merged commit fa7fb7d into microsoft:main Jun 21, 2022
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.

2 participants