-
Notifications
You must be signed in to change notification settings - Fork 227
Update the pid/ppid in transport metadata if they have changed #825
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
💔 Tests FailedExpand to view the summary
Build stats
Test stats 🧪
Test errorsExpand to view the tests failures
Log outputExpand to view the last 100 lines of log output
|
@beniwohli this could still use a review when you have a moment. |
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.
LGTM, but maybe we can simplify a bit
A test for this would also be great :)
Also made build_metadata a public method in the client
@beniwohli I realized when writing the test that if we generate metadata at start_thread time, we would lose any metadata explicitly set in the Transport kwargs. This was problematic for testing, but also begged the question: why would we bother generating it at all before we create the Transport, when it's going to be immediately overwritten when we start the thread? So now we store any metadata that is sent into the Transport as This is a much larger-scale change now, and I'm interested in your opinion. |
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.
Generally, I like the idea, but I'm struggling to find a use case for overriding/modifying the metadata per transport. Did you have something in mind?
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.
LGTM!
Just flakey tests failing. Merging. |
…ic#825) * Update the pid/ppid in transport metadata if they have changed * Only update the process metadata if it already exists * Fix os.getppid for windows python 2.7 * Add to changelog * Rebuild metadata on pid change Also made build_metadata a public method in the client * Generate metadata only during start_thread * Remove metadata kwarg from transport completely * Remove metadata :param
…ic#825) * Update the pid/ppid in transport metadata if they have changed * Only update the process metadata if it already exists * Fix os.getppid for windows python 2.7 * Add to changelog * Rebuild metadata on pid change Also made build_metadata a public method in the client * Generate metadata only during start_thread * Remove metadata kwarg from transport completely * Remove metadata :param
…ic#825) * Update the pid/ppid in transport metadata if they have changed * Only update the process metadata if it already exists * Fix os.getppid for windows python 2.7 * Add to changelog * Rebuild metadata on pid change Also made build_metadata a public method in the client * Generate metadata only during start_thread * Remove metadata kwarg from transport completely * Remove metadata :param
Related issues
Closes #792