search

substrait-io / substrait-validator

Apache License 2.0
8 stars 8 forks source link

chore(ci): Fix python release cycle #297

Closed wackywendell closed 3 weeks ago

wackywendell commented 3 weeks ago

This fixes the build for Python releases, which will hopefully resolve #296.

We may need to change the tag for v0.1.0 for this to work; alternatively, we could release v0.1.1 if we can't change the tag.

wackywendell commented 3 weeks ago

Hm, I think there are slashes in github.ref, which aren't allowed.

The suggestion here is to explicitly use the matrix components, so I might do that. I'll push some more on this tomorrow.

wackywendell commented 3 weeks ago

Blargh, that was harder than I expected to get CI to work. I'm going to clean up, rebase and squash a bit, and then mark this ready for review.

wackywendell commented 3 weeks ago

@mbrobbel - thanks for the quick reviews! It looks like we're understanding how Github namespaces artifacts, jobs, and workflows differently, so let's try and resolve that - see comments above.

Additionally - from my own perspective, adding the "dry run" features is useful for this PR to see if it 'works', but means adding a bunch of unnecessary artifacts to every pull request, and also the above note "@wackywendell deployed to VALIDATOR_RELEASE — with GitHub Actions" doesn't seem right. So I'm dropping the dry run "feature", so that going forward PRs will continue to not upload, download, or attempt to 'release' artifacts.

Does that make sense? What are your thoughts?

mbrobbel commented 3 weeks ago

@mbrobbel - thanks for the quick reviews! It looks like we're understanding how Github namespaces artifacts, jobs, and workflows differently, so let's try and resolve that - see comments above.

Additionally - from my own perspective, adding the "dry run" features is useful for this PR to see if it 'works', but means adding a bunch of unnecessary artifacts to every pull request, and also the above note "@wackywendell deployed to VALIDATOR_RELEASE — with GitHub Actions" doesn't seem right. So I'm dropping the dry run "feature", so that going forward PRs will continue to not upload, download, or attempt to 'release' artifacts.

Does that make sense? What are your thoughts?

Ah, I was under the assumption that we also upload artifacts for non-tag workflows, but I missed https://github.com/substrait-io/substrait-validator/blob/0ffbb83f7849848eb7d31b577319cf31bb8fc483/.github/workflows/python.yml#L99.

Now it makes sense, it was just failing because we bumped but never tested after that, and it had nothing to do with the other job that ran for the same commit.

Anyway, I'm happy to merge this PR and release 0.1.1. What are you thoughts? Do you want to prepare the 0.1.1 PR?

wackywendell commented 3 weeks ago

Sure - I'll make a 0.1.1 release PR, if you can merge this and then release that. Thanks!