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

PR workflow fixes #54

Merged
merged 1 commit into from
Nov 22, 2024
Merged

PR workflow fixes #54

merged 1 commit into from
Nov 22, 2024

Conversation

jikortus
Copy link
Contributor

@jikortus jikortus commented Nov 8, 2024

  • Add missing dependencies (even though they only relate to lanabot/runtime/installation/hub/keyboard/layouts.py, it seems better to add them instead of ignore the import errors in pylint)
  • Ensure no containers using the base image are left before removing the image. This should address the current failures of the deployment phase in PR workflow, even though I have no idea how there could still be some leftover containers that are actually using the image.

@jikortus jikortus force-pushed the workflows-checks-update branch from 107f77c to 437e3de Compare November 8, 2024 18:50
podman kill anabot
podman image rm quay.io/centos/centos:stream9
Copy link
Collaborator

Choose a reason for hiding this comment

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

Couldn't this be done simply by podman image rm --force ... ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it should work the same, I think. I may try to resort to the force deletion, but I wonder why the image still doesn't get deleted by the current solution, despite it not being used by any container (as you can see in the workflow script output). @velezd, you know most likely much more about the GitHub workflows - is it possible that this is a 'feature' for some reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jstodola so I tried to just enforce the image removal as you suggested and it works fine (as you can see in the checks below). However, just FYI, I think that even if we didn't do any cleanup at all, it shouldn't actually cause any mess, because the workflows use a fresh VM every time, which makes sense after all:

Each workflow run executes in a fresh, newly-provisioned virtual machine.

@jikortus jikortus force-pushed the workflows-checks-update branch from 437e3de to 7f64ce0 Compare November 20, 2024 09:22
Copy link
Collaborator

@jstodola jstodola left a comment

Choose a reason for hiding this comment

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

It looks good to me. We can remove the cleanup part if it keeps causing issues in the future.

@jikortus jikortus merged commit c2bddd5 into main Nov 22, 2024
3 checks passed
@jikortus jikortus deleted the workflows-checks-update branch November 22, 2024 14:27
@jikortus
Copy link
Contributor Author

Thanks for the review!

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.

2 participants