Skip to content

Conversation

@cmccabe
Copy link
Contributor

@cmccabe cmccabe commented Jul 8, 2021

No description provided.

@cmccabe cmccabe requested review from hachikuji and mumrah July 8, 2021 00:40
New versions of pip apparently removed support for Python 3.5, the
version of Python our ducker image is using. They used to support it,
and now they only support newer versions of Python.

We will eventually have to upgrade the version of Python in the image.
That will probably mean moving to a different Ubuntu base image.
However, for now, using pip 20.3.4 seems to work and is easier.
@cmccabe cmccabe force-pushed the fix-python-pip-version-mess branch from dcac06d to 918b4a7 Compare July 8, 2021 15:42
@lbradstreet
Copy link
Contributor

I hit this same problem and was able to resolve it with docker system prune -a to make sure it grabbed a later openjdk image where this is not a problem. See the discussion in #10661.

Maybe we can modify this PR to instead provide a more helpful error message?

@cmccabe cmccabe changed the title MINOR: fix ducker-ak Dockerfile issue MINOR: Hint about "docker system prune" when ducker-ak build fails Jul 8, 2021
echo_and_do() {
local cmd="${@}"
echo "${cmd}"
${cmd}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic through must_do, which we were doing previously also redirects output and checks the command return. Should we do something similar?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well... I made this new function because the other one was checking the output and exiting if it was nonzero. That didn't allow me to print the error message I wanted. So doing the same thing here would defeat the point :)

If bash shell was a better language, I could add a lambda argument to the existing function and have the lambda return what to print on failure. But shell doesn't really do closures, so it would turn into kind of a mess.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I guess some people would say that I'm doing things too much the Java way here, and that the kewl bash way of doing this would be

set -x
<run command>
set +x

But it still feels kind of like a mess since, does set +x change $? ?
Sigh.

@kamalcph
Copy link
Contributor

kamalcph commented Jul 8, 2021

There is also a tip to run the command with --no-cache flag on error. Why isn't it exposed?

@kamalcph
Copy link
Contributor

kamalcph commented Jul 8, 2021

Also, can you update the label from ducker.type to ducker.creator here?

@cmccabe
Copy link
Contributor Author

cmccabe commented Jul 8, 2021

@kamalcph : Thanks, that is a good find. I agree we should update the label from ducker.type to ducker.creator. Can you make a separate PR for that? I can review it quickly.

@kamalcph
Copy link
Contributor

kamalcph commented Jul 8, 2021

@kamalcph : Thanks, that is a good find. I agree we should update the label from ducker.type to ducker.creator. Can you make a separate PR for that? I can review it quickly.

PR: #10499

@cmccabe
Copy link
Contributor Author

cmccabe commented Jul 8, 2021

PR: #10499

@kamalcph: Thanks for that. Can you make a more minimal PR that just changes ducker.type to ducker.creator?

We're getting so close to the release, and I just don't have time to review a lot of bash changes (to be 100% honest)

@cmccabe
Copy link
Contributor Author

cmccabe commented Jul 8, 2021

There is also a tip to run the command with --no-cache flag on error. Why isn't it exposed?

I agree --no-cache should probably appear in the help output somewhere. Let's tackle this in a follow-on. I don't know if it would help with the current issue that many people are seeing with their cache.

@cmccabe cmccabe merged commit 5a88a59 into apache:trunk Jul 8, 2021
@cmccabe cmccabe deleted the fix-python-pip-version-mess branch July 8, 2021 16:58
cmccabe added a commit that referenced this pull request Jul 8, 2021
@kamalcph
Copy link
Contributor

kamalcph commented Jul 8, 2021

@cmccabe
Updated #10499, can you take another look?

xdgrulez pushed a commit to xdgrulez/kafka that referenced this pull request Dec 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants