Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
System tracing provider for tracing via native probes (BPF) #1288
base: master
Are you sure you want to change the base?
System tracing provider for tracing via native probes (BPF) #1288
Changes from 1 commit
e5b6c29
5d38039
543a0f0
362286f
6101a3f
091d08a
1c42607
eb5c346
0a4f7f6
189bf7c
552e754
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Following the same reasoning as the other comment, unless we have some reason to avoid it I'd install all the extras in the Dockerfile - that way the
seldonio/mlserver
image is always ready as-is to enable tracepoints.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.
Happy to do that if the decision (given above comments) is to install all extras. I don't know if pip has a simple way of doing that here, I might still have to build a list.
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.
We can also see if Poetry supports an
all
extras target? Otherwise we can always add one.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.
(and / or just hardcode a
pip install mlserver[extras1, extras2, etc.]
)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.
Poetry currently supports --all-extras on install but not export (this is tracked in python-poetry/poetry-plugin-export#45). I've added an
everything
extra. I've intentionally avoided the nameall
as that might be reserved in the future by poetry/pip, but we can move to it once there is enough support.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.
TBH I would just use
all
as it's more standard. If poetry decides to always add one automatically we can just remove it and use Poetry's built-in target.