update to modern docker package #3
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lukapeschke
requested changes
Dec 11, 2023
entrypoint.py
Outdated
| def __init__(self): | ||
| super(MainObj, self).__init__() | ||
| super(Main, self).__init__() |
There was a problem hiding this comment.
Suggested change
| super(Main, self).__init__() | |
| super().__init__() |
Heh, Python has changed a lot since I wrote this :)
There was a problem hiding this comment.
Yes, I didn't notice this change.
There was a problem hiding this comment.
Because Main is not extend any explicit base class, so super().__init__() is optional and recommend to ignore it IMO.
entrypoint.py
Outdated
Comment on lines
37
to
43
| # ignore the end "# buildkit" comment | ||
| step = re.sub("\s*#.+$", "", step) | ||
| if "#(nop)" in step: | ||
| to_add = step.split("#(nop) ")[1] | ||
| else: | ||
| to_add = ("RUN {}".format(step)) | ||
| # step may contains "/bin/sh -c ", just ignore it | ||
| to_add = "{}".format(step.replace("/bin/sh -c ", "")) |
There was a problem hiding this comment.
Maybe this should be changed in another PR ? The results with the current docker image and one built from your branch are quite different:
With the current image:
❯ docker run --rm -v '/var/run/docker.sock:/var/run/docker.sock' lukapeschke/dfa 21ab84251c7b
FROM lukapeschke/dfa:latest
ADD file:093f0723fa46f6cdbd6f7bd146448bb70ecce54254c35701feeceb956414622f in /
CMD ["/bin/sh"]
RUN /bin/sh -c apk add --update python3 wget \
&& wget -O - --no-check-certificate https://bootstrap.pypa.io/get-pip.py | python3 \
&& apk del wget \
&& pip3 install -U docker-py \
&& yes | pip3 uninstall pip
COPY file:df2a17029ab5f655279e92b51f29046f182eb0a4a5457ad06b2338387bcbbd20 in /root
ENTRYPOINT ["/root/entrypoint.py"]
With the image build from your PR:
❯ docker run --rm -v '/var/run/docker.sock:/var/run/docker.sock' updated-image 21ab84251c7b
FROM lukapeschke/dfa:latest
/bin/sh -c
/bin/sh -c
apk add --update python3 wget \
&& wget -O - --no-check-certificate https://bootstrap.pypa.io/get-pip.py | python3 \
&& apk del wget \
&& pip3 install -U docker-py \
&& yes | pip3 uninstall pip
/bin/sh -c
/bin/sh -c
There was a problem hiding this comment.
I updated the code 😄 It should fixed in the following commits.
liudonghua123
force-pushed
the
modern-docker-package
branch
from
f54ac7f to
83c9b7f
Compare
lukapeschke
reviewed
Dec 11, 2023
lukapeschke
approved these changes
Dec 11, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.