Repository navigation
refactor: make client almost client again - #1666
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1666 +/- ##
==========================================
- Coverage 96.47% 96.34% -0.14%
==========================================
Files 49 51 +2
Lines 3804 3832 +28
==========================================
+ Hits 3670 3692 +22
- Misses 134 140 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
62fc048 to
3f9a08d
Compare
|
|
||
| if (nt_conf := configuration.numtracker) is not None: | ||
| if configuration.env.metadata is not None: | ||
| from blueapi.service.numtracker import NumtrackerClient |
There was a problem hiding this comment.
If we need to import it anyway, is there any real benefit to wrapping the initial import in the TYPE_CHECKING check?
I see the circular import error when importing the client eagerly. Were there other issues with numtracker being in the utils package instead of service? Lazy imports to avoid circular references feels like we're working around doing something wrong.
There was a problem hiding this comment.
numtracker is a server only thing so I do feel that it should be in the service module, The main problem with keeping numtracker in utils is cli also gets the httpx which is what I'm trying to avoid.
|
@ZohebShaikh please address comments and conflicts raised here. Thank you. |
cb7aa1d to
a16e800
Compare
tpoliaw
left a comment
There was a problem hiding this comment.
Is it possible to run the client tests without the server group being installed? With this set up, all development work and CI tests are going to have server dependencies available and I can see it being easy for the client to fail at runtime with a missing dependency.
12eaaec to
834626f
Compare
Move FastAPI, aiohttp, graypy, httpx, tiled and related packages to the optional `server` extra so the client can be installed without them. Move session handling into blueapi.client.session, numtracker into blueapi.service, and the Python environment code into blueapi.service.environment. Update docs, the Dockerfile and tests.
b5ea1ab to
79ab49f
Compare
I'm trying to resolve this issue #1414
By the following changes
client.session, environment handling to service.environment)
ie. Separating out what is client only and what is server only functionality
Once this is merged, I would like to go further in the split by doing the following:-
bluesky, ophyd-async,event-model,opentelemetry-distro,observability-utilsto server only