Import Reorg Plan #4
Conversation
| This also in parts shows up in import times. To run a single test file for a | ||
| utils module (`test_rust.py`) pytest will spend 0.2 seconds in test execution vs | ||
| 2 seconds in import time. While imports are so far acceptable, the bigger issue | ||
| caused by it is that it increases the total amount of surface that we need to | ||
| consider for test execution. |
There was a problem hiding this comment.
when I did my quick "what tests what" demo using coverage I completely ignored the module scope because of the difficulties here. this work would also better enable us to do some automated selective test execution (internal notion doc)
|
|
||
| # Drawbacks | ||
|
|
||
| The drawback of this change is that imports become more verbose: |
There was a problem hiding this comment.
personally I find the more verbose imports to be better -- it's much easier to know where something comes from rather than having to git grep '^class OrganizationMember\b' and such
There was a problem hiding this comment.
I’m with you there. I see it as advantage :P
There was a problem hiding this comment.
I generally agree. The only time it's useful to be able to import everything at once is when I'm in the shell. Not a good enough reason to not do this though.
There was a problem hiding this comment.
I'm in the shell. Not a good enough reason to not do this though.
Could we add shell startup code that generates more ergonomic glob modules that make it possible to do from sentry.models import User only in the console?
There was a problem hiding this comment.
Yeah. I think we can and probably should add some lazy import code there afterwards for shell usage.
Co-authored-by: anthony sottile <[email protected]>
| As such all code related to the processing pipeline should be moved into a clear | ||
| structure and have a largely independent test setup (think moving all of processing | ||
| related logic to `sentry.services.processing` or `sentry_processing` for better | ||
| enforcability). |
There was a problem hiding this comment.
I think we should put thought into how services overlap & intersect as they will. Core concepts like Organizations and Projects are everywhere, and as we deepen connections between issues, performance, profiling and replays we create tension with the independence of each service.
In a previous role we had a 'service API' which was the public interface that other services inside the application could use. A similar approach has some documentation at shopify.
There was a problem hiding this comment.
Agreed. I would like to handle that as a separate independent proposal though.
|
I merged this as a tentative decision. Before doing something beyond what is done to date I will revisit this with a few folks. |
This RFC proposes a preliminary refactoring of the Sentry monolith in order
to support later compartmentalization into services by eliminating mass
re-exports in
sentry.appandsentry.models. The goal is to make thedependency tree easier to understand in order to break it into separate
services by making modules the boundary of our dependencies.
In other words we want to be able to say that if module A depends on module B
it depends on the entirey of it. Today this is violated by a
sentry.appand
sentry.models.Rendered RFC