-
Notifications
You must be signed in to change notification settings - Fork 10
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
poc for local + hot reloaded worker + shared #29
Conversation
Codecov Report
@@ Coverage Diff @@
## main #29 +/- ##
=======================================
Coverage 98.59% 98.59%
=======================================
Files 359 359
Lines 26284 26284
=======================================
Hits 25914 25914
Misses 370 370
Flags with carried forward coverage won't be shown. Click here to find out more.
This change has been scanned for critical changes. Learn more |
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.
could you add --ignore=shared
to the addopts
list in worker's pytest.ini
?
Signed-off-by: joseph-sentry <[email protected]>
- Rename build-dev to dev - Creation of build.base_dev target - Requirements.dev is requirements.txt with the dependency on shared removed - Use of docker build arg to know when to use requirements.dev Signed-off-by: joseph-sentry <[email protected]>
Signed-off-by: joseph-sentry <[email protected]>
e18e193
to
6aadc77
Compare
6aadc77
to
c223a5f
Compare
One thing that is not very clear to me (possibly because I haven't really worked with git submodules), how do you specify what is the shared SHA when deploying? Or these changes don't modify the deploy to staging/prod at all, just the local development setup? |
in your local directory you will see |
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.
I'll say looks good, and I'm fine with the changes. But please update README.md with the instructions that are in the PR comment 🙏
6b2398e
to
1186251
Compare
1186251
to
9b155ec
Compare
@matt-codecov just curious what the status of this PR was |
there was a strange CI issue, was waiting for the GHA migration to settle and just haven't gotten back to it since |
alrighty, i think this is ready for review
there are more ways to build
worker
images now:the latter two tag their resulting docker images with
codecov/worker-dev
instead ofcodecov/worker
so if you switch you will want to update yourdocker-compose.yml
or whatever.the downside of hot-reloading shared is, when you initially start the container, it takes an extra couple minutes to fully build shared. it'll look like it froze during installation, but if you come back with coffee it should be running. hot-reloading will be very speedy, it's just that initial container startup.
to toggle hot-reloading shared, just run
make dev
ormake dev-shared
and restart thecodecov/worker-dev
container.to work on or update shared:
this same approach should work for API as well. if it's approved here, joseph or i will do it there as well.
the submodule part
instead of installing
shared
from GitHub at a specified hash, make it agit submodule
with that same hash checked out and install it withfile:./shared#egg=shared
with just this, it's easier to work with shared. change
shared
, rebuild your worker image. no more pushing WIPs to GitHub to copy the hash. however, it's not enough to hot-reloadthe hot-reload part
we install
watchmedo
which is a util that does hot-reloading. if you change a python file it'll re-runworker.sh
and your changes will take effectbut shared is special. shared is imported from the
site-packages
directory rather than the submodule. we need another step to hot-reload sharedhot-reloading shared
we can perform an "editable" install of
shared
withpip install -e ./shared
. this binds the installed module to the original, in-tree sources. however, if we do that while building the docker images, it'll bind the module to the copies of the sources inside the docker container.instead, we want to run it after we've started the container. at that point, we've bind-mounted our own copy of
worker
/shared
into the container, and when we runpip install -e ./shared
, it'll bind to our copy.for reasons unknown, this only wants to work after a non-editable install has also been performed.
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.