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

chore: Updates action versions and add dependabot to check monthly for updates #824

Merged
merged 2 commits into from
May 30, 2024

Conversation

stumpylog
Copy link
Contributor

Updates the actions which could be done with little to minimal changes.. Includes a monthly dependabot check for actions now.

This is motivated by recent updates to the Node 16 availability for runners: https://github.blog/changelog/2024-05-17-updated-dates-for-actions-runner-using-node20-instead-of-node16-by-default/

Changelogs for review:

Other possible updates:

  • pypa/cibuildwheel@v2.18.0
    • musllinux was updated to default to 1.2 (1.1 / Alpine 3.12 is long EoL now). This would require build script updates, etc and might be considered breaking
    • Seemed to be something with MacOS too, it doesn't finish repairing the wheel

Builds seem to be fine:

@dvarrazzo
Copy link
Member

I see that you addressed actions/upload-artifact#478, that's good thank you very much.

Can I see the result of an upload? I would like to know if this will result in dozens of files to download separately or if it still possible to download all of them with one click. In psycopg 2 we had a script to do that, so if it becomes a need in psycopg 3 too we can resume it and adapt it. It was dropped in this commit: psycopg/psycopg2@0b01ded

@stumpylog
Copy link
Contributor Author

stumpylog commented May 18, 2024

If you have a look at the runs from my fork, it is a bunch of artifacts that would be downloaded individually.

If you'd like, there is a possibility to merge them all I can look into, so it would be a single artifact in the end.

Edit: It was hard to link from my phone. For some examples: https://github.com/stumpylog/psycopg/actions/runs/9136644822 or https://github.com/stumpylog/psycopg/actions/runs/9135244783

It's a pretty long list in the binary build :/

@dvarrazzo
Copy link
Member

Hello, thank you, yes, I found your job.

You can understand well that it is not feasible that to make a release I should click and download manually all these items. And I am not interested in a workflow pushing artifacts automatically from github to pypi: it's too dangerous.

I would be ok to either have all the artifacts packaged in a single archive or to have a script to download all of them.

update-types:
- "major"
- "minor"
- "patch"
Copy link
Member

Choose a reason for hiding this comment

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

Please make sure that there is a newline at the end of the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

@stumpylog
Copy link
Contributor Author

Yep, I figured that might be the case. I'm testing what the merge will look like with the worse case workflow. That will give each workflow a single artifact/zip.

@stumpylog
Copy link
Contributor Author

Alright, all the test runs in master are still going as a double check, but I've updated to merge all produced artifacts together at the end. It will look like this, which is pretty much the same result as currently.

I chose the if: always() so even if some jobs fail, any produced artifacts still end up merged together.

@dvarrazzo
Copy link
Member

dvarrazzo commented May 19, 2024 via email

@stumpylog
Copy link
Contributor Author

Certainly up to you, I'm not familiar enough with the release process here. My thought was someone might want to download the artifact to investigate something, and it would be clear enough that jobs failed it wouldn't be used. But I'm happen to change that


# Enable updates for GitHub Actions
- package-ecosystem: "github-actions"
target-branch: "dev"
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, just checked this... is this correct? Shouldn't it be master?

uses: actions/upload-artifact/merge@v4
with:
name: psycopg-binary-artifact
delete-merged: true
Copy link
Member

Choose a reason for hiding this comment

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

Please add an end of line to every file.

@dvarrazzo
Copy link
Member

Hello! Sorry, I was about to merge this MR because, looking at your artifacts it seems to do the right thing. So I've force-pushed to squash two temporary commits, but then noticed a couple of issues. So please pull before changing them.

Thank you very much!

@dvarrazzo
Copy link
Member

(I have fixed the typo causing lint error on master, so a further rebase will clean the failed check).

@stumpylog
Copy link
Contributor Author

No problem, I'll get those fixed up when I'm back at a computer

@dvarrazzo dvarrazzo merged commit cabf9a1 into psycopg:master May 30, 2024
43 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

2 participants