Separate conda-store app instance from config

[soapy1]

pre req for resolving #1005

Description

This PR refactors conda_store_server/app.py to split the config portion and app instance portion of the class. The goal of this refactor is more clearly delineate responsibility between the 2 classes/pieces of functionality. Eventually, this should also make how to approach changes to the config more clear.

2 new objects are introduced:

• conda_store_config:CondaStore
  • this is the config class for conda store
  • it uses traitlets to get config
  • it's only responsibility is to read the config file and provide that info to conda_store:CondaStore
• conda_store:CondaStore
  • this has common app functions like get_settings, set_settings, etc
  • it gets it's configuration from conda_store_config:CondaStore
  • it's responsibility is to have shared instance information and provide common functionality

Pull request checklist

• Did you test this change locally?
• Did you update the documentation (if required)?
• Did you add/update relevant tests for this change (if required)?

[netlify]

✅ Deploy Preview for conda-store canceled.

Name	Link
🔨 Latest commit	e89d60a
🔍 Latest deploy log	https://app.netlify.com/sites/conda-store/deploys/675b8e93272b730008c4665c

[netlify]

✅ Deploy Preview for conda-store canceled.

Name	Link
🔨 Latest commit	533ceb1
🔍 Latest deploy log	https://app.netlify.com/sites/conda-store/deploys/677c0821dad88a0008d368fd

[peytondmurray]

A few general questions about this:

• conda_store_server._internal.server.app.CondaStoreServer still inherits from traitlets.config.Application. Is this needed anymore? Like does traitlets actually do anything for the CondaStoreServer after this change?
• There are still configuration options in the CondaStoreServer class. Does it make sense to move these to CondaStoreServer.config? For someone unfamiliar with conda-store, it will be hard to understand why these config values live on the class while others live on .config.

I'm definitely in favor of this, but I'll avoid merging until we get feedback from @trallard and possibly others.

[peytondmurray]

Would it be possible to add some documentation about what this is? What does this class do, how is it different from the traitlets/FastAPI CondaStoreServer application, etc would be very helpful here.

Really this seems like a celery worker manager - maybe a different name could provide a clearer idea what this class is for. Or maybe the webserver should be named differently 🤔 What do you think?

[soapy1]

conda_store_server._internal.server.app.CondaStoreServer still inherits from traitlets.config.Application. Is this needed anymore? Like does traitlets actually do anything for the CondaStoreServer after this change?

Yep, it's still needed in order for there to be CondaStoreServer config object. That config object is described in the docs, it provides the ability to specify server level configuration like url_prefix or port. That's in contrast to the CondaStore config which is separated out in this PR from the CondaStore class. The CondaStore config allows users to set stuff like the storage class and specify the build directory. This config is used by both the conda store server and conda store worker.

There are still configuration options in the CondaStoreServer class. Does it make sense to move these to CondaStoreServer.config? For someone unfamiliar with conda-store, it will be hard to understand why these config values live on the class while others live on .config.

Ya, I think we can do that. But I would say it's out of scope for this PR. I think conda-store should be working towards:

• not using traitlets for config
• being able to provide config from plugins

in order to achieve these 2 ends we'll need to separate out configuring the CondaStoreServer instance from starting the webserver. But, I think not much is gained from doing that before conda-store has a more robust way of reading configs and making them available in the right places. This PR is meant to be a stepping stone to get to that point.

[peytondmurray]

Awesome, thanks for the explanation, I'm all clear on this now 🚀