-
Notifications
You must be signed in to change notification settings - Fork 13
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
[nodejs] allow mounting local dd-trace-js as weblog volume #2777
Conversation
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 like the idea of assembling the volumes, but I also wonder how much we gain. I mean looking for the balance of keeping the code easy to read, its complexity and homogeneity (we run apps in different languages).
This is by far the biggest pain point of working with system tests (except for them being slow), so I'd say it's warranted especially since it doesn't really increase complexity all that much. As for homogeneity, I would agree but only in the sense that this should also be done for other languages if possible, although this is out of scope of this first PR. |
I addressed his comments and he just went on vacation for a month
Don't worry, he just sent me all his pending reviews. |
7936b2b
to
9a170ea
Compare
af48ad0
to
84cb275
Compare
It looks like the CI failure is also happening on other branches and so it's unrelated to this PR. |
fda1b95
to
60cb2bb
Compare
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.
Don't know much about the python internals but otherwise LGTM
@cbeauchesne PR is ready for one last look! |
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.
All good, thank you for tackling this.
Motivation
Right now we have to
npm pack
the local code, copy the resulting archive tosystem-tests/binaries
, and rebuild weblog for every single code change before we can rerun the tests.By allowing to instead mount the local dd-trace-js folder as a volume on the container, we can simply rerun the tests on code changes without having to go through any of the above.
Changes
Allow mounting local dd-trace-js as weblog volume.
Workflow
codeowners
file quickly.🚀 Once your PR is reviewed, you can merge it!
🛟 #apm-shared-testing 🛟
Reviewer checklist
[<language>]
, double-check that only<language>
is impacted by the changebuild-XXX-image
label is present