mountetna / monoetna

mono-repository version of etna projects
GNU General Public License v2.0
1 stars 0 forks source link

This PR attempts to fix a build issue within swarm-keeper #1286

Closed dtm2451 closed 6 months ago

dtm2451 commented 6 months ago

Details: A new build error surfaced today after @dtm2451 merged a new PR into master, erroring here in a line of the swarm-keeper Dockerfile. Investigation led towards a suggestion that the issue may come from the alpine image starting point newly adopting PEP668. Indeed, there was a new alpine image version pushed 4 days ago, with major version number incremented, between our last successful build-and-push job run 5 days ago, and the current run.

Attempted fix then:

An alternative fix we can try if we want to continue receiving alpine image updates would be to instead try adding --break-system-packages to the problematic pip call.

dtm2451 commented 6 months ago

Review Q: Thoughts about whether to 1) pin to the older alpine image (the change currently made) versus 2) let the apt&pip package managers possibly become out of sync as was presumably the behavior in the previous version anyway?

dtm2451 commented 6 months ago

Confirmed! With previous alpine image, we only get a warning, "WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv" here

maflaven commented 6 months ago

Review Q: Thoughts about whether to 1) pin to the older alpine image (the change currently made) versus 2) let the apt&pip package managers possibly become out of sync as was presumably the behavior in the previous version anyway?

I think the proper solution would be having some sort of lockfile (like poetry uses). Slightly prefer what the new warning suggests, but whatever sees us through the kubernetes transition is enough imo seeing as swarm-keeper will likely go out of use at that time.

image
dtm2451 commented 6 months ago

I think the proper solution would be having some sort of lockfile (like poetry uses).

For sure, but I don't think it's worth building that for this image because, as you also note, this image is gonna go away soon!

Slightly prefer what the new warning suggests

I've also been slightly leaning this way the more I thought about it. I'll revert back to the 1st commit where that was the solution chosen.