search

multi-py / python-oso

Multiarchitecture Docker Containers for Python and OSO
MIT License
10 stars 1 forks source link

django-oso PYTHONPATH #1

Open victormartingarcia opened 2 years ago

victormartingarcia commented 2 years ago

Hey!

I am copying the oso multi-py packages to my Dockerfile like stated in the doc:

# python-oso 🔴 
COPY --from=ghcr.io/multi-py/python-oso:py3.8-slim-LATEST /usr/local/lib/python3.8/site-packages/* /usr/local/lib/python3.8/site-packages/
COPY --from=ghcr.io/multi-py/python-oso:py3.8-slim-LATEST /opt/oso /opt/oso
# end python-oso

I ran into the issue that, even django-oso pypi was successfully installed, I couldn't import it on my code (ModuleNotFoundError: No module named 'django_oso')

Listing all modules available to import, django-oso was not there. To my surprise, /opt/oso/languages/python/django-oso/sqlalchemy-oso WAS there even I have no intention to use it.

FYI I managed to make it work adding django-oso to PYTHONPATH, but in my opinion it is a bit of unexpected behaviour:

# python-oso fixed ✅ 
COPY --from=ghcr.io/multi-py/python-oso:py3.8-slim-LATEST /usr/local/lib/python3.8/site-packages/* /usr/local/lib/python3.8/site-packages/
COPY --from=ghcr.io/multi-py/python-oso:py3.8-slim-LATEST /opt/oso /opt/oso
ENV PYTHONPATH="${PYTHONPATH}:/opt/oso/languages/python/django-oso/"
# end python-oso
tedivm commented 2 years ago

Thanks! This is a bug, I'll get an updated version out with the additional packages.

gj commented 2 years ago

@tedivm what do you think would be best for letting folks enable either {oso} or {oso,sqlalchemy-oso} or {oso,django-oso}? I can't imagine someone would ever want both sqlalchemy-oso and django-oso, but maybe I'm not being open-minded enough 😄

tedivm commented 2 years ago

So I originally thought that we'd have to build each package, but @victormartingarcia managed to get things working by just changing the python path so all of the packages must be present (and the make build_THING step is just linking to it). So nothing we do should really change the build size of the images, just whether they're accessible.

That gives a few different options to consider-

  1. Link all the subpackages so they're automatically present. The downside here as that people would get all of the packages whether they liked it or not.
  2. Link none of the packages, but provide the documentation needed for developers to add their specific packages.
  3. Add an onbuild trigger with a small script, so downstream containers can basically control this with args or envvars.
  4. Create separate containers for the different packages based on the tag. This will only really work if I update the Github Teams plan that this organization is on, as I'm already running into some concurrency issues. I'm probably going to have to do this eventually anyways though.

I'm likely not going to be able to make a decision on this until this weekend (it's Thanksgiving here and I'll only partially available until the weekend) but all of these are simple changes that should be possible to push out early next week. Until then we can keep the conversation going so we pick the best option available.

gj commented 2 years ago

I'm not in love with (1) for the downside you mentioned. (2) seems like a great first step. (3) sounds like it might be easier to implement than (4) in the short-term, but this is the first time I've seen that ONBUILD directive so take my opinion w/ a grain of salt 😄