This repository was archived by the owner on Sep 18, 2023. It is now read-only.
DM-37921: Experimental client-server mono repo layout #193
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonathansick
force-pushed
the
tickets/DM-37921
branch
from
0224e68 to
c8902e1
Compare
Move the FastAPI application into a server directory; allowing us to add a "client" library package directory off the repo root in the future. I've chosen to keep the Dockerfile in the repo's right, even though it's really for the server. I think this will be helpful when installing the client package dependency during the docker builds.
FastAPI 0.91.0 updates how it handles middleware, and this seems to be generating errors like > Cannot add middleware after an application has started We might need to look at updating our Safir middleware if this isn't an error on FastAPI's part.
jonathansick
force-pushed
the
tickets/DM-37921
branch
from
c8902e1 to
79fd45b
Compare
The library is set up as a namespace package; right now this uses rubinobs.square.clients; but RSP clients might use lsst.rsp as their base namespace package instead.
jonathansick
force-pushed
the
tickets/DM-37921
branch
from
c8bf61c to
bd0dc4e
Compare
Caveats: - To do a local install with tox we needed to disable hashing for the requirements.txt files to avoid the "require hashes" mode. I don't know how ot selectively disable this for tox. Potentially we could generate multiple sets of requirements.txt files; with hashes and without? - The client library currently needs to depend on Safir to get base models from Safir.
This is useful for VSCode users to enable detection of the client library codebase when looking at imports in the server code. Not even installing the client into the current environment being used by the editor solves this.
Use the hashed dependencies in the Docker build, but use unhashed dependencies for tox to work around the requirement for hashed dependencies with the local installation.
It's a separate job to support running as a matrix with potentially more Python versions
jonathansick
force-pushed
the
tickets/DM-37921
branch
from
092b9a5 to
9c0e878
Compare
Middleware needs to be set up in the application constructor rather than in a startup event handler.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.