-
Notifications
You must be signed in to change notification settings - Fork 28
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
Use Azure Storage Tables for collection and container configuration #48
Conversation
Brining in the type stubs for cachetools caused mypy to complain about unknown types for the key function
Also run flake8 on pccommon, which wasn't happening
Also refactor CommonConfig to use pydantic settings. Create a table setup for collection configuration and container configuration. Use cachetools to cache the configuration.
Encode collection configuration and container configuration (which was hardcoded) as JSON. This can be used to populate the initial table structure in deployment as well, after which this test data will diverge from production settings.
Also account for environment prefix for DEBUG that change with refactor to use BaseSettings in CommonConfig
c25477a
to
f6069f8
Compare
Enable configuration of TTL
2708a3d
to
a44c544
Compare
Also fix setup_azurite
This was being used inconsistently.
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.
This looks good! I was able to test it locally with the explorer and it fetches the mosaicInfo as expected, but the subsequent tile requests error (see below).
The only other comment is that it looks like the cli is used for dumping/loading a table-wide file. Is there a mode to target a particular rowkey only? I'm thinking of the use case of tweaking render params for a new collection and just working in a single file rather than the dump of the whole table.
This was left over from a previous implementation, should have been cleaned up.
The dump is table-wide, but the load would handle a single collection (JSON with a top level single key). But it's a good point that the workflow would likely to be to dump a single collection, tweak, and the load that same file. Will change. |
- name: "PCAPIS_CONTAINER_CONFIG__ACCOUNT_KEY" | ||
value: "{{ .Values.tiler.storage.account_key }}" | ||
- name: "PCAPIS_CONTAINER_CONFIG__TABLE_NAME" | ||
value: "{{ .Values.tiler.storage.container_config_table_name }}" | ||
- name: APP_INSIGHTS_INSTRUMENTATION_KEY |
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.
Might need to update this to PCAPIS_APP_INSIGHTS_INSTRUMENTATION_KEY
- PCAPIS_CONTAINER_CONFIG__ACCOUNT_NAME=devstoreaccount1 | ||
- PCAPIS_CONTAINER_CONFIG__ACCOUNT_KEY=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw== | ||
- PCAPIS_CONTAINER_CONFIG__TABLE_NAME=containerconfig | ||
|
||
# Used for logging and metrics | ||
- APP_INSIGHTS_INSTRUMENTATION_KEY |
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.
Might need to update this to PCAPIS_APP_INSIGHTS_INSTRUMENTATION_KEY
default=None, | ||
env=APP_INSIGHTS_INSTRUMENTATION_KEY, |
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.
Does setting env
like this avoid the env_prefix
?
Description
This PR removes the hard-coded configuration for collections in favor of configuration stored in Azure Storage Tables.
It refactors the
CommonConfig
class, moving it to usepydantic.BaseSettings
and adding table configuration for the tables holding the collection and container configurations.It adds Azurite into the development environment, which is used to hold dev data in those tables.
It also refactors some of the usage of caching, using cachetools.cachedmethod with instance caches.
It adds a storage account to the terraform deployment, and a CLI that can be used to populate the tables with configuration.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Checklist:
Please delete options that are not relevant.