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

Add workflow to bump package after dependabot merge #984

Merged

Conversation

Odilhao
Copy link
Member

@Odilhao Odilhao commented May 8, 2024

No description provided.

@Odilhao Odilhao force-pushed the rpm/develop-bump-package-after-merge branch 2 times, most recently from 8c27ec1 to cc14e30 Compare May 8, 2024 15:47
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I'd still argue for an approach where requirements.txt isn't strict, but rather pins to ranges, for example using pulp ~= 3.39.0 (which also allows installing 3.39.1). Then you'd install that and see if you need to update packaging. Otherwise requirements.txt goes out of sync with the rest of the repository all the time.

An additional benefit is that it can also work for stable branches: just pin to the .z stream.

In foreman-packaging we have:

This builds up a list of updates to start. Each branch has a script to list the available updates. In practice that's https://github.com/theforeman/foreman-packaging/blob/rpm/develop/list_updatable_packages and https://github.com/theforeman/foreman-packaging/blob/deb/develop/scripts/list_updatable_packages.

For pulpcore-packaging, that could take https://github.com/theforeman/pulpcore-packaging/blob/rpm/develop/automation/_generate_deps.sh and format it in compatible way.

Then we have generated a matrix with jobs to update. You can see how we do that for RPMs here:

https://github.com/theforeman/foreman-packaging/blob/afb6040d279916cb79065468e719ea6495c163fc/.github/workflows/bump_packages.yml#L33-L36

For pulpcore-packaging you may want to take a different approach and group them, but in my experience with foreman-packaging it's better to have them split so you can review things individually.

- name: Check if package can be updated
if: github.event_name == 'push' && github.head_ref == 'rpm/develop'
run: |
set +e
Copy link
Member

Choose a reason for hiding this comment

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

I'd strongly advise against big shell scripts in YAML. Create a separate shell script and run that. If it's limited to just GHA, store it in .github. That way you can modify it with proper syntax highlighting and tools like shellcheck.

.github/workflows/update-pulp-packages.yml Outdated Show resolved Hide resolved
.github/workflows/update-pulp-packages.yml Outdated Show resolved Hide resolved
@Odilhao
Copy link
Member Author

Odilhao commented May 8, 2024

I'd still argue for an approach where requirements.txt isn't strict, but rather pins to ranges, for example using pulp ~= 3.39.0 (which also allows installing 3.39.1). Then you'd install that and see if you need to update packaging. Otherwise requirements.txt goes out of sync with the rest of the repository all the time.

We have dependabot enabled now, and it's working to keep with z-streams of pulpcore and pulp-rpm, we need this due to VCRs on Katello that can get broken when we do major upgrades.

I'll refactor later this week and move from being one big script inside of YAML to one standalone script.

I do have plans to revive the _generate_deps.sh, unfortunately right now we have packages that still use setup.py and other that don't use it anymore or never used, and the spec generation is not straightforward in the dependency side. For now we will only focus on getting the core components of pulp up to date, to later implement as Foreman is doing right now.

@Odilhao Odilhao force-pushed the rpm/develop-bump-package-after-merge branch 3 times, most recently from 9f54f03 to 04e24f6 Compare July 23, 2024 16:58
@Odilhao
Copy link
Member Author

Odilhao commented Jul 24, 2024

@ekohl I think we are now in a better shape for another review. 😄

@Odilhao
Copy link
Member Author

Odilhao commented Jul 24, 2024

Let me tag @evgeni as well, I heard that he loves GH-actions

@Odilhao Odilhao force-pushed the rpm/develop-bump-package-after-merge branch from 0a37b09 to 5d210d4 Compare July 29, 2024 15:08
@Odilhao Odilhao force-pushed the rpm/develop-bump-package-after-merge branch from 5d210d4 to 94f9f28 Compare July 30, 2024 12:34
Copy link
Contributor

@zjhuntin zjhuntin left a comment

Choose a reason for hiding this comment

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

Logic of this looks good to me!

@Odilhao
Copy link
Member Author

Odilhao commented Jul 31, 2024

Merging and YOLO

@Odilhao Odilhao merged commit 621822b into theforeman:rpm/develop Jul 31, 2024
2 checks passed
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.

None yet

3 participants