-
Notifications
You must be signed in to change notification settings - Fork 3k
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
docker: do not remove build dep which might be needed by extension #8291
Conversation
But if an extension has a dependency, shouldn't the extension declare its own dependency so that it is installed? We can't expect the CLI to know all the dependencies an extension might need and carry them. |
I might be misunderstanding the issue, but we should consult with @williexu |
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 agree with @tjprescott. Indiscriminately adding build dependencies will bloat our image more than in helps support extensions. We should continue to work with the authors of extensions that aren't working to ensure they've accurately described all of their dependencies.
The workaround for now is: There is no simple patch can be done at the extension end, afaik. To maintain the sandbox model, CLI installs all dependencies to extension’s own folder, which means the build tools must be around. This also means, in a nutshell, that any extensions has azure-mgmt-* or msrest-azure called out in setup.py will be broken. For context, most extensions don’t have any dependencies specified at all in setup.py, guess that is why they are still surviving. For the payload increase of not removing the build deps, I would not worry too much, once this issue #7387 gets addressed, we would sure find some balances back. |
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 chatted about this offline for quite a while. We agreed that being able to install extensions is enough of a core-scenario that it's worth increasing the size of the image to facilitate it.
Fix #8284
Note: the image size increases from 497MB to 666MB, but extension installation must work so build dep has to stay
The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).
I adhere to the Command Guidelines.