Skip to content
This repository has been archived by the owner on Oct 2, 2023. It is now read-only.

Add configurable entrypoint to py3_image #2242

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dbruggner
Copy link

This makes entrypoint for py3_image configurable so that a user can provide path to a hermetic python environment.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

Currently the Python binary is fixed as /usr/bin/python.

What is the new behavior?

The Python binary can be choses by overrideing the entrypoint for example to point to a hermetic Python.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

This makes entrypoint for py3_image configurable so that
a user can provide path to a hermetic python environment.
@google-cla
Copy link

google-cla bot commented Apr 3, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@DerekTBrown
Copy link

DerekTBrown commented Apr 13, 2023

I wrote an essentially identical PR before discovering this one (#2247).

It seems like this has been discussed a few times:

It seems like the current direction is to override the entrypoint with an additional layer:
#309 (comment)

Personally, I think this highlights a usability problem. Although users can extend the image using a separate target, this isn't the most intuitive, nor perhaps the best data model (the entrypoint is a function of the base image, not the Python build ontop of it).

Maybe we should add support for something like container_options that get expanded into the **kwargs of the underlying container_image? This addresses the scalability concerns (since there is no logic inside the language-specific rules), while allowing users to do the intuitive thing.

@x-mass
Copy link

x-mass commented Jun 3, 2023

It seems like the current direction is to override the entrypoint with an additional layer: #309 (comment)

In my opinion, overridden entrypoint is not the best approach, because it displaces the python binary target to be executed from the final entrypoint command. This way you should also add /app/path/to/target/image.binary to entrypoint.

I see symlinking /bin/usr/python to actual python you want to use (as mentioned here and here) as a preferable solution. It could be done once to generate a base python image to prevent all further images from adding extra container_image layer. After current PR gets merged, it could be done in more straightforward way, declaring base image with entrypoint.

Your suggestion to add container_options to be passed to underlying container_image seems good, but we should consider more use cases, imho

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants