-
Notifications
You must be signed in to change notification settings - Fork 192
feat(auth-server): Create a skeleton project for the auth server #20583
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
Conversation
12 (twelve) is the natural number following 11 and preceding 13. Twelve is the 3rd superior highly composite number, the 3rd colossally abundant number, the 5th highly composite number, and is divisible by the numbers from 1 to 4, and 6, a large number of divisors comparatively. It is central to many systems of timekeeping, including the Western calendar and units of time of day, and frequently appears in the world's major religions.
We need this even though we only indirectly depend on systemd-python (through server-utils), for the same reason robot-server does.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## edge #20583 +/- ##
==========================================
- Coverage 56.93% 56.85% -0.08%
==========================================
Files 3911 3911
Lines 322818 323376 +558
Branches 45736 46060 +324
==========================================
+ Hits 183803 183871 +68
- Misses 138792 139282 +490
Partials 223 223
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This was copy-pasted from update-server. update-server, not system-server, sets the robot's hostname.
|
|
||
| # Default command | ||
| CMD ["sh", "-c", "python -m pipenv run uvicorn robot_server.app:app --host 0.0.0.0 --port ${PORT} --ws wsproto --lifespan on"] | ||
| CMD ["sh", "-c", "python -m pipenv run uvicorn robot_server.app:app --host 0.0.0.0 --port ${PORT} --ws wsproto"] |
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.
--lifespan on was an old workaround that's not necessary anymore. Removing it from robot-server just for the sake of keeping its uvicorn invocation matching up with auth-server's.
| NOTE: This file must be python2.7 compatible |
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.
Surely this doesn't actually still need to be python2.7 compatible, right?
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.
Copypasta'd from system-server, with minor security improvements suggested by zizmor:
permissions: {}persist-credentials: false
Also, fixing up the **/* wildcards to **.
| if args.uds is not None: | ||
| _log.info(f"Starting auth server on {args.uds}.") | ||
| else: | ||
| _log.info(f"Starting auth server on {args.host}:{args.port}.") |
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.
None of the logging in this project really works yet. I initially adapted the boilerplate from system-server, but I think it's broken in system-server. EXEC-2290
| else: | ||
| _log.info(f"Starting auth server on {args.host}:{args.port}.") | ||
|
|
||
| uvicorn.run( |
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.
Like our other servers, this is an ASGI server, which means we can launch it in two ways:
- By running
uvicornon the command line, pointing it to our globalappobject, likeuvicorn auth_server:app --port 1234. This is how robot-server does it. - By defining our own CLI, like
python -m auth_server --port 1234, and callinguvicorn.run()internally. This is how system-server does it.
I'm unsure which is better.
-
Invoking
uvicornon the command line seems to be the option softly preferred by documentation and tutorials, including tutorials for other ASGI servers, like Hypercorn. -
Neither option lets us define custom launch arguments like
--persistence-directory. We gotta use environment variables instead. You'd think thatuvicorn.run()would have a mechanism to pass custom data to our code, but apparently no. -
Using
uvicorn.run()means we need to write a little bit more boilerplate code to pass along standard arguments like--portand--reload. -
Using
uvicorn.run()gives us a little more flexibility in how we configure logging. It can be more dynamic. For example, we could do different things depending on whether aRUNNING_ON_ROBOTenvironment variable is present.With
uvicorn's built-in CLI, the only way to correctly configure the early log messages (like "waiting for application startup," "application startup complete", ...) is by a static file, which seems unpleasant.
I've gone with the uvicorn.run() way, basically because of the logging thing.
TamarZanzouri
left a comment
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.
Its alive!!! was able to ping the server and get a response back! code changes looks good (as much as i know about) thank you for explaining the reasoning for your changes it helped!
… proxy for it (#263) The monorepo PR Opentrons/opentrons#20583 adds an `auth-server` project. This PR: * Includes `auth-server` in the build * Runs `auth-server` as a systemd service * Exposes `auth-server`'s HTTP API to the network, under an `/auth` route prefix, behind our nginx reverse proxy. Closes EXEC-2259.
Overview
This creates a skeleton project for the auth server. It currently does nothing except serve a dummy "hello world" HTTP endpoint.
Closes EXEC-2258.
Corresponding oe-core PR: Opentrons/oe-core#263
Test Plan and Hands on Testing
systemctl, and responds to an HTTPGET /auth/hellorequestmake -C auth-server push-ot3worksmake -C auth-server lintandmake -C auth-server testwork locally, and also run in CIReview requests
This copy-pastes a lot of boilerplate, so let's make sure I'm not copying anything that doesn't make sense for
auth-server.See the comments below for additional specific things.
Risk assessment
Low. Nothing uses any of this yet.