-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide Debian image #34
Conversation
type=registry,ref=pgautoupgrade/pgautoupgrade:${{ matrix.pg_target }}-${{ matrix.operating_system.flavor }} | ||
type=registry,ref=pgautoupgrade/pgautoupgrade:${{ matrix.pg_target }}-${{ matrix.operating_system.flavor }}${{ matrix.operating_system.version }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so for bookworm
, no version is set in the GitHub Actions matrix. If I am not mistaken, this should work as GitHub Actions will substitute the missing version
as an empty string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets try it out. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not want to add a bunch of if
statements to the Dockerfile, so I opted to add another Dockerfile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good, it makes sense to me.
My feeling is that this will probably be a thing that we try out and see how well it works (ie over a few weeks/months), and from that we can decide if it's best to keep two Dockerfiles or have it all in one, or something else. 😄
@@ -13,6 +13,9 @@ jobs: | |||
runs-on: ubuntu-latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noticed this and the thought "should we be using Debian instead?" popped up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
anecdotally, i would say ubuntu seems to be more standard and common for things like this. personally, i tend to think "ubuntu unless i have a reason not to"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that used to be the previous default for a lot of people, though it doesn't seem to be the case as much any more.
For me, when Canonical started including advertising in their cli is when I personally stopped defaulting to Ubuntu for anything and begun defaulting to Debian instead.
For an existing build process though (like here), it's unlikely to be worth the effort to go about changing the distribution due to trivial preference stuff like that. So yeah, no worries. 😉
This looks really good. 😄 I've been trying to figure out how we should potentially deal with the problem of us having an Alpine based image by default, whereas the official PostgreSQL image uses Debian. My best thought so far is that we might need to use a 2nd repository on Docker that defaults to the Debian image. Maybe pg_autoupgrade_debian or similar. Then we could have instructions in each repos pointing people to the _debian repo for Debian based PostgreSQL, and the first repo for Alpine based ones. Better ideas welcome of course. 😄 |
so the build generally worked on the |
Heh Heh Heh. Running out of disk space is unexpected. 😉 |
This PR adds a Debian image (currently only Bookworm) to our build.
it is a bit strange. so the official Postgres image provides an
alpine
tag, but also version-specific ones (alpine3.19
andalpine3.20
). For Debian, there are tags forbookworm
, but no genericdebian
tag. I followed this pattern in this PR.