Add extra dependencies to docker container #1073
Conversation
| @@ -6,13 +6,10 @@ This guide explains how to implement LDAP authentication using an external serve | |||
|
|
|||
| ### Install System Packages | |||
|
|
|||
| !!! danger | |||
| FIXME(jathan): With `wheel` packages asserted, let's make doubly sure these development libraries even need to be installed anymore. | |||
There was a problem hiding this comment.
Were you able to confirm that these libraries are still needed in the current installation? I see you've added them to the Dockerfile - were there failures without adding them?
There was a problem hiding this comment.
yes there definitely were
There was a problem hiding this comment.
If I may add some inputs: on Alpine, while openldap-dev is required to build the package, only libldap seems to be required at runtime (and outside of ldap, postgresql-dev zlib-dev jpeg-dev libffi-dev at build time, libpq zlib libjpeg libffi at run time)
There was a problem hiding this comment.
@u1735067 thanks for the input, unfortunately, we don't support alpine. I'd be curious if you are able to test using the Debian containers for runtime dependencies.
|
|
||
| ```no-highlight | ||
| $ echo django-storages >> $NAUTOBOT_ROOT/local_requirements.txt | ||
| $ echo "nautobot[storages]" >> $NAUTOBOT_ROOT/local_requirements.txt |
There was a problem hiding this comment.
Feels weird to me to be putting nautobot into local_requirements.txt in any way shape or form. Maybe this change should be reverted? Compare to similar verbiage in authentication/ldap.md that you haven't updated in this way.
There was a problem hiding this comment.
I agree, but I think there is a fundamental difference between pip install django-storages and pip install nautobot[remote_storage] I believe the latter will follow the version requirements specified by the nautobot package which are driven by the pyproject.toml when the package is built. Installing django-storages could potentially have version conflicts.
That said it's still odd adding nautobot in an additional requirements file. This might bring the larger issue up in that local_requirements.txt could potentially drive dependency issues with Nautobot itself. Should we recommend some kind of poetry environment be setup locally, I'm not sure how that would look exactly...
|
The |
| @@ -104,13 +108,20 @@ pycryptodome = "~3.10.1" | |||
| pyuwsgi = "~2.0.19.1.post0" | |||
| # YAML parsing and rendering | |||
| PyYAML = "~5.4.1" | |||
| # Social authentication core | |||
| social-auth-core = {extras = ["openidconnect", "saml"], version = "~4.1.0", optional = true} | |||
There was a problem hiding this comment.
Does marking social-auth-core as optional actually make sense given that social-auth-app-django is non-optional and it in turn requires social-auth-core? Is this just for the optional openidconnect and saml extras? I ask because the poetry.lock here looks suspicious (https://github.com/nautobot/nautobot/pull/1073/files#diff-f53a023eedfa3fbf2925ec7dc76eecdc954ea94b7e47065393dbad519613dc89R1731) - note the empty dependencies for openid and saml:
[extras]
all = ["mysqlclient", "django-auth-ldap", "django-storages"]
ldap = ["django-auth-ldap"]
mysql = ["mysqlclient"]
openid = []
remote_storage = ["django-storages"]
saml = []
There was a problem hiding this comment.
Thanks for finding this Glenn! it looks like we need to wait for python-poetry/poetry#3913 to have the separate openid/saml extras, for now I'll combine them into a single sso
|
Fixes #888. |
Oops sorry I missed it, I think it's definitely in the spirit of this PR, I'll get it updated ASAP. |
Fixes: #1068
This PR adds the optional dependencies described in the docs to poetry as optional dependencies and extras to configure for Nautobot. A custom extra
allwill combine all of the optional dependencies. The docker container is updated to include theallextra to bundle all optional components in the docker container.