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

Remove LRU cache #54

Merged
merged 3 commits into from
May 5, 2022
Merged

Remove LRU cache #54

merged 3 commits into from
May 5, 2022

Conversation

alukach
Copy link
Contributor

@alukach alukach commented May 5, 2022

Using the LRU cache does not offer any benefit, as the settings are initialized just once at app startup:

settings = ApiSettings()

async def connect_to_db(
app: FastAPI, settings: Optional[PostgresSettings] = None
) -> None:
"""Connect to Database."""
if not settings:
settings = PostgresSettings()

cache_config = CacheSettings()

They additionally don't allow settings to be provided manually by keyword arguments. If it is deemed preferable to have the LRU present, an alternative solution would be to alter the cached function to pass through kwargs to the settings class.

Using the LRU cache does not offer any benefit, as the settings are on initialized once at app startup:

https://github.com/stac-utils/titiler-pgstac/blob/780f4f8815f9349f72bb7805fad1306bb4412291/titiler/pgstac/main.py#L31
https://github.com/stac-utils/titiler-pgstac/blob/780f4f8815f9349f72bb7805fad1306bb4412291/titiler/pgstac/db.py#L12-L17
https://github.com/stac-utils/titiler-pgstac/blob/780f4f8815f9349f72bb7805fad1306bb4412291/titiler/pgstac/dependencies.py#L20

They additionally don't allow settings to be provided manually by keyword arguments.  If it is deemed preferable to have the LRU present, an alternative solution would be to alter the cached function to pass through kwargs to the settings class.
alukach added a commit to alukach/titiler-pgstac that referenced this pull request May 5, 2022
To enable users to manually specify settings (e.g. pass them in via keyword arguments), cached settings should pass along keyword arguments.

This is an alternative solution to stac-utils#54.
@alukach alukach mentioned this pull request May 5, 2022
@vincentsarago vincentsarago self-requested a review May 5, 2022 22:31
@vincentsarago
Copy link
Member

@alukach could you please update the changelog 🙏

@alukach
Copy link
Contributor Author

alukach commented May 5, 2022

@vincentsarago

@vincentsarago vincentsarago merged commit eef93e2 into stac-utils:master May 5, 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