PsyNeuLink icon indicating copy to clipboard operation
PsyNeuLink copied to clipboard

ci: update on-tag pypi release action

Open kmantel opened this issue 1 year ago • 5 comments

kmantel avatar Oct 23 '24 02:10 kmantel

This PR causes the following changes to the html docs (ubuntu-latest-3.11):

diff -r docs-base/ModulatoryProjection.html docs-head/ModulatoryProjection.html
272c272
< <a class="reference internal" href="Port.html"><span class="doc">Port</span></a> are listed in the Port’s <a class="reference internal" href="Mechanism.html#id7" title="psyneulink.core.components.mechanisms.mechanism.Mechanism_Base.mod_afferents"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">mod_afferents</span></code></a> attribute.</p>
---
> <a class="reference internal" href="Port.html"><span class="doc">Port</span></a> are listed in the Port’s <a class="reference internal" href="ParameterPort.html#psyneulink.core.components.ports.parameterport.ParameterPort.mod_afferents" title="psyneulink.core.components.ports.parameterport.ParameterPort.mod_afferents"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">mod_afferents</span></code></a> attribute.</p>
diff -r docs-base/Port.html docs-head/Port.html
616c616
< <code class="xref any docutils literal notranslate"><span class="pre">PathWayProjections</span></code> (listed in its <a class="reference internal" href="Mechanism.html#id6" title="psyneulink.core.components.mechanisms.mechanism.Mechanism_Base.path_afferents"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">path_afferents</span></code></a> attribute) as the variable for its
---
> <code class="xref any docutils literal notranslate"><span class="pre">PathWayProjections</span></code> (listed in its <a class="reference internal" href="#psyneulink.core.components.ports.port.Port_Base.path_afferents" title="psyneulink.core.components.ports.port.Port_Base.path_afferents"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">path_afferents</span></code></a> attribute) as the variable for its
diff -r docs-base/TransferFunctions.html docs-head/TransferFunctions.html
2361c2361
< <p><a class="reference internal" href="#psyneulink.core.components.functions.transferfunctions.Exponential.bounds" title="psyneulink.core.components.functions.transferfunctions.Exponential.bounds"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">bounds</span></code></a> – specifies the lower and upper limits of the result;  if there are none, the attribute is set to
---
> <p><a class="reference internal" href="OptimizationFunctions.html#psyneulink.core.components.functions.optimizationfunctions.GradientOptimization.bounds" title="psyneulink.core.components.functions.optimizationfunctions.GradientOptimization.bounds"><code class="xref any py py-attr docutils literal notranslate"><span class="pre">bounds</span></code></a> – specifies the lower and upper limits of the result;  if there are none, the attribute is set to

...

See CI logs for the full diff.

github-actions[bot] avatar Oct 23 '24 02:10 github-actions[bot]

This PR causes the following changes to the html docs (ubuntu-latest-3.11):

No differences!

...

See CI logs for the full diff.

github-actions[bot] avatar Oct 25 '24 02:10 github-actions[bot]

The main problem with the workflow is that the GA security model doesn't allow jobs to depend on other jobs that include secrets. So the 'test-release' part cannot depend on 'create-python-dist', as long as the latter uses secrets to upload the packages. Either the upload needs to split off to its own job, or we need to find a way to work around the restriction. There also isn't a good way to install pnl wheel on a 32bit Python. The pnl-test CI jobs hack around the torch requirement by removing it (and mdf) from the requirements file, but this is not possible for wheels/packages. we'd need to add something like a notorch extra if we want wheels/sdist working on 32-bit Python

The idea behind this test workflow was to check if the sdist/wheel packages were created correctly (to catch issues like https://github.com/PrincetonUniversity/PsyNeuLink/issues/2040). I don't think we need the full copy of the special jobs from the main test files. Since we release universal wheel, we should be good enough to just check that both packages install OK on all supported platofrms

Not sure if there's a quick fix for the existing release scripts. I used to test them on a local repo creating test tags (and skipping uploads), because of GA security constraints.

It might be worth using this opportunity to adjust how CI should work combined with release scripts. The regular CI jobs can install from wheels instead of editable local folder as is done currently. The wheels are already created in every pnl-test CI job so the steps only need to be shuffled around so the wheels package is used during testing. The release script could then just test that installation via both sdist and wheel works and they both install the same files. It can just wait for the regular CI job on master instead of re-running the tests.

jvesely avatar Oct 25 '24 13:10 jvesely

The logs from the previous run have expired, but I do see "Skip output 'sdist' since it may contain secret.". On my fork, it doesn't seem to have a problem with this https://github.com/kmantel/PsyNeuLink/actions/runs/11527544088. Is that what you meant by how you tested it? Where it's possibly different repo or secrets settings that are letting it through. Or maybe the security model has changed since this was last looked at?

I can see the benefit of reworking CI to use wheels; if shifting to that, I think I'd just go ahead with the next release first without any CI changes.

kmantel avatar Oct 26 '24 02:10 kmantel

The logs from the previous run have expired, but I do see "Skip output 'sdist' since it may contain secret.". On my fork, it doesn't seem to have a problem with this https://github.com/kmantel/PsyNeuLink/actions/runs/11527544088. Is that what you meant by how you tested it? Where it's possibly different repo or secrets settings that are letting it through. Or maybe the security model has changed since this was last looked at?

I can see the benefit of reworking CI to use wheels; if shifting to that, I think I'd just go ahead with the next release first without any CI changes.

Thanks. hm, it looks like the rules now allow the existing flow. I think it makes sense to fix/resurrect the script independently of changes to the main CI jobs. I assumed that the test-release job will fail, but if it runs, it should do something useful. Without syncing the special jobs in the matrix, LGTM.

jvesely avatar Oct 26 '24 12:10 jvesely

This PR causes the following changes to the html docs (ubuntu-latest-3.11):

No differences!

...

See CI logs for the full diff.

github-actions[bot] avatar Oct 31 '24 05:10 github-actions[bot]

This PR causes the following changes to the html docs (ubuntu-latest-3.11):

No differences!

...

See CI logs for the full diff.

github-actions[bot] avatar Oct 31 '24 05:10 github-actions[bot]

This PR causes the following changes to the html docs (ubuntu-latest-3.11):

No differences!

...

See CI logs for the full diff.

github-actions[bot] avatar Nov 05 '24 03:11 github-actions[bot]