ci: Fix python upload

[max-sixty]

It was trying to upload directories: https://github.com/PRQL/prql/actions/runs/9432185880/job/25981724946#step:3:54

This limits it to the wheel and source extensions

[kgutwin]

Looks like it didn't upload anything :(

https://github.com/PRQL/prql/actions/runs/9432813673/job/25983251820#step:3:38

Edit: Maybe it should be a single star rather than a double star? */*{.whl,.tar.gz} not sure...

[eitsupi]

As I commented before, I believe it fails because it does not merge the artifacts.
#4444 (comment)

[eitsupi]

If I understand correctly, the solution here is to merge the artifacts and download them to the target directory of the maturin action.
But @max-sixty did not agree with me so I am not sure if this is correct.

[max-sixty]

OK sorry, clearly I am wrong in some way!

Though re the merging, I was relying on, from here, my emphasis:

For most cases, this may not be the most efficient solution. See the migration docs on how to download multiple artifacts to the same directory on a runner. This action should only be necessary for cases where multiple artifacts will need to be downloaded outside the runner environment, like downloads via the UI or REST API.

[max-sixty]

Edit: Maybe it should be a single star rather than a double star? */*{.whl,.tar.gz} not sure...

Yeah something like this seems likely — I added some file listings in #4568

I'm not completely confident it's not the artifacts but it does have files — the previous one failed because there were too many files...

[max-sixty]

It looks like maturin's action just doesn't pick up globs at all: https://github.com/PRQL/prql/actions/runs/9439404299/job/25997680563?pr=4568

Though when it was just *, it did pick up globs! So maybe they've done their own globbing implementation??

[eitsupi]

Though re the merging, I was relying on, from here, my emphasis:

I do not know how important efficiency is to you as it is something that is rarely executed.
In any case, why stick to the glob pattern without using the suggested migration docs listed there?

[max-sixty]

In any case, why stick to the glob pattern without using the suggested migration docs listed there?

But the suggested migration docs state those should only be used "outside the runner environment, like downloads via the UI or REST API.", no?

And the files are there — they do download — it's the maturin action that doesn't seem to find them given a glob...

[eitsupi]

But you didn't follow the migration doc. Did you?
https://github.com/actions/upload-artifact/blob/main/docs/MIGRATION.md#multiple-uploads-to-the-same-named-artifact

[eitsupi]

But the suggested migration docs state those should only be used "outside the runner environment, like downloads via the UI or REST API.", no?

In my opinion, it makes more sense to adopt a method that is sure to succeed, even if it is inefficient.

[max-sixty]

But you didn't follow the migration doc. Did you? https://github.com/actions/upload-artifact/blob/main/docs/MIGRATION.md#multiple-uploads-to-the-same-named-artifact

But I think I did! They're all different names now. And they're all listed when we list the files after downloading the artifacts — https://github.com/PRQL/prql/actions/runs/9439404299/job/25997680563?pr=4568#step:3:7

What would you look at to assess whether the artifact download isn't working vs. the maturin glob isn't working?

[eitsupi]

That is not what I have been pointing out for a month.
It used to be that all whl files were downloaded into one directory, but now, as you can see, that is not the case. (I am quite annoyed that you continue to ignore my opinion, even though I have been pointing out for a month that this will fail.)

[eitsupi]

@max-sixty Please read the documentation carefully. You are not making the changes as described in the Migration Guide.

jobs:
  upload:
    strategy:
      matrix:
        runs-on: [ubuntu-latest, macos-latest, windows-latest]
    runs-on: ${{ matrix.runs-on }}
    steps:
    - name: Create a File
      run: echo "hello from ${{ matrix.runs-on }}" > file-${{ matrix.runs-on }}.txt
    - name: Upload Artifact
-     uses: actions/upload-artifact@v3
+     uses: actions/upload-artifact@v4
      with:
-       name: my-artifact
+       name: my-artifact-${{ matrix.runs-on }}
        path: file-${{ matrix.runs-on }}.txt
  download:
    needs: upload
    runs-on: ubuntu-latest
    steps:
    - name: Download All Artifacts
-     uses: actions/download-artifact@v3
+     uses: actions/download-artifact@v4
      with:
-       name: my-artifact
        path: my-artifact
+       pattern: my-artifact-*
+       merge-multiple: true
    - run: ls -R my-artifact

[max-sixty]

It used to be that all whl files were downloaded into one directory, but now, as you can see, that is not the case.

This is true! This does seem to be the problem — maturin can't use any glob apart from * and that doesn't work with anything in a nested directory.

(I am quite annoyed that you continue to ignore my opinion, even though I have been pointing out for a month that this will fail.)

I apologize. I agree you said it wouldn't work and it indeed doesn't work.

I had thought you were suggesting to use https://github.com/actions/upload-artifact/blob/main/merge/README.md, which seems different from the most recent comment. I am genuinely trying to find out which part isn't updated...

[eitsupi]

Chronologically, the ability to merge multiple artifacts was not implemented immediately after the release of v4. These features and the Migration Guide were added later.
I think my comment a month ago was based on old knowledge.

I think the migration guide now accurately reflects the situation - I think you need to review the workflow when it was working well in v3 and migrate to v4.

[max-sixty]

Chronologically, the ability to merge multiple artifacts was not implemented immediately after the release of v4. These features and the Migration Guide were added later.
I think my comment a month ago was based on old knowledge.

I see.

FWIW I realize that often I move too fast and you find a mistake of mine. Given that, I should recognize that you are likely to be correct when you point something out...

---

Let's see how #4577 goes; it should put everything in the same dir; otherwise we can do a separate merge step etc

[max-sixty]

It almost worked, but now we get an auth error; I think because it's using Charlie's old auth.

When I get pypi/support#4179 fixed, I'll update with my auth...

[eitsupi]

Glad to hear it.
Perhaps after updating the secret you can simply re-run the failed job and release it? (I'd rather not see any more extra tags added if that's possible)

Another question is whether PyPI can configure releases from a tied GitHub repository without a secret.
That is how it was configured below?
https://github.com/PRQL/pyprql/blob/4514dea1372b7c6a3b506978481d4b7725b11a15/.github/workflows/release.yaml#L64-L65

[max-sixty]

Another question is whether PyPI can configure releases from a tied GitHub repository without a secret.
That is how it was configured below?
https://github.com/PRQL/pyprql/blob/4514dea1372b7c6a3b506978481d4b7725b11a15/.github/workflows/release.yaml#L64-L65

Yes this is much better, thanks.

I still think I need to get access to my PyPI account (I'm very confused why I'm locked out — I logged in for other projects a few months ago, and I don't see any changes in my 1Password history...). So when that happens I'll try to switch to the new method

[kgutwin]

It almost worked, but now we get an auth error; I think because it's using Charlie's old auth.

When I get pypi/support#4179 fixed, I'll update with my auth...

It looks like both account recovery and PEP 541 requests at PyPI are severely delayed (the backlog is large and triage can take months). I'm wondering -- is there something we could do in the interim to get the latest release up on PyPI somehow? Could we continue to use prql-python until the pending PEP 541 request is complete?

[max-sixty]

Yes, sorry about this. I'm really not sure how the 2FA error happened.

Good idea re prql-python, but we can't publish to that either yet — only me & Charlie are maintainers, Charlie isn't online these days, and I can't login.

We could publish to prqlc-python-temp or similar if you want it sooner?

It can be installed with pip install git+https://github.com/PRQL/prql.git#egg=prqlc-python&subdirectory=prqlc/bindings/prqlc-python but requires a compiler so less good for broad distribution...

[kgutwin]

I understand! I read through several discussion threads and know that this is a fairly common problem. It also prompted me to double-check my own PyPI credentials and recovery codes 😅 Here's to hoping your account recovery is speedy.

For me personally, I think we shouldn't publish to prqlc-python-temp. I found a way to build the python artifacts using the release workflow in our fork, and pushed them to a temporary S3 bucket. It isn't pretty but it's working for the time being.

We should probably, though, create a tracking issue for this so that other people are aware that the Python release will be lagging, pending resolution of at least one of these PyPI requests. Maybe if there are others who need a recent release, we can do something like a temporary publish.

[eitsupi]

Sorry for the delay, I created the issue: #4943