Skip to content
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

Switch to CentOS Stream 9 as base image #60

Merged

Conversation

anoopcs9
Copy link
Collaborator

No description provided.

@obnoxxx
Copy link
Contributor

obnoxxx commented Jan 20, 2023

@anoopcs9 - why "do-not-merge"?

@phlogistonjohn
Copy link
Collaborator

phlogistonjohn commented Jan 20, 2023

Seems pretty reasonable. My main concern is that with the change in base os we're changing the versions of python that get used for running the tests. That said we can probably take this and iterate from that point... either getting py3.10 (or up) on centos stream 9 or making the containerfile flexible to run on either a fedora or centos base.

Therefore I'm leaning on taking this and then tweaking it but let me meditate on a little bit before approving.

@anoopcs9
Copy link
Collaborator Author

@Mergifyio rebase

@mergify
Copy link

mergify bot commented Jan 22, 2023

⚠️ This pull request got rebased on behalf of a random user of the organization.
This behavior will change on the 1st February 2023, Mergify will pick the author of the pull request instead.

To get the future behavior now, you can configure bot_account options (e.g.: bot_account: { author } or update_bot_account: { author }.

Or you can create a dedicated github account for squash and rebase operations, and use it in different bot_account options.

@mergify
Copy link

mergify bot commented Jan 22, 2023

rebase

✅ Branch has been successfully rebased

@phlogistonjohn
Copy link
Collaborator

FWIW, I was working on something else and wanted to try f37 instead of f36. My workaround for the build process was to disable the rpm install and use the python wheel - which doesn't care about the python abi version.

Copy link
Collaborator

@phlogistonjohn phlogistonjohn left a comment

Choose a reason for hiding this comment

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

LGTM. I think I will want to make this work on multiple base distros but that can be done later. Thanks!

@obnoxxx obnoxxx merged commit af63900 into samba-in-kubernetes:master Jan 22, 2023
@obnoxxx
Copy link
Contributor

obnoxxx commented Jan 22, 2023

@anoopcs9 apologies: I merged despite the do-not-merge label because @phlogistonjohn has approved.... what now?

why has mergify not kicked in?

@anoopcs9 anoopcs9 deleted the switch-to-stream9-base-img branch January 23, 2023 04:18
@anoopcs9
Copy link
Collaborator Author

anoopcs9 commented Jan 23, 2023

@anoopcs9 apologies: I merged despite the do-not-merge label because @phlogistonjohn has approved.... what now?

Actually I had something else in mind while setting the do-not-merge label i.e, installing python3-sambacc(built with CentOS Stream 9) on Fedora 36. samba-container is layered with sambacc:latest and fedora:36 and thus I was not sure of any dependency issues/errors while installing python3-sambacc on Fedora 36. And now with this merged we encounter some installtion issues, see https://github.com/samba-in-kubernetes/samba-container/actions/runs/3982840129/jobs/6827649451

@anoopcs9 anoopcs9 removed the do-not-merge Hold on label Jan 23, 2023
@phlogistonjohn
Copy link
Collaborator

@anoopcs9 apologies: I merged despite the do-not-merge label because @phlogistonjohn has approved.... what now?

why has mergify not kicked in?

Mergify didn't kick in because not all the conditions were met. It would have ignored Anoop's do-not-merge label because of a typo. But it still would not have merged because the rule, summarized in english is:
Merge any PR that has 2 or more approvals and no requested changes. If the submitter is a maintainer then merge if the PR has one approval and is over two weeks old. Currently the only maintainers are me (@phlogistonjohn ) and @obnoxxx.

I was going to follow up on the miantainers thing later this week.

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.

3 participants