ci: Minor reorganizations #1190

LecrisUT · 2023-12-04T10:25:57Z

Removed concurrency where it wasn't necessary. Only top-level workflow needs it
Harden the CI workflows
Reorganize workflow order
Update actions. Normally dependabot is used to update these actions (also node/python dependencies) but the dependabot PRs can get quite noisy. Do you want it to be setup though?
~~Setup a full release CD~~
- Slightly blocked by: Cannot use this action in a chain re-actors/alls-green#25
- The workflow would be more complicated and would require re-converging the version numbering, let's do this in a next step
Reconfigure codecov action
- Relaxed the upload to codecov to only occur in PR and when pushing
- This is a rather weird issue, maybe it's some race condition or the fact that it is run through a fork. Hopefully in v4 this is resolved:
  "Unable to locate build via Github Actions API" error with v3 uploader codecov/codecov-action#1138
  public repo: token needed or not? codecov/codecov-action#1035
  Tokenless support in the new CLI (for public repos and forks) codecov/feedback#112
Other minor improvements

github-actions · 2023-12-04T10:26:12Z

Thank you for making this pull request.

Did you know? You can try it on Binder: or .

Also, the version of Jupytext developed in this PR can be installed with pip:

pip install git+https://github.com/LecrisUT/jupytext.git@ci/organize

(this requires nodejs, see more at Developing Jupytext)

codecov · 2023-12-04T10:52:17Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0606f03) 97.73% compared to head (1870d19) 96.74%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1190      +/-   ##
==========================================
- Coverage   97.73%   96.74%   -0.99%     
==========================================
  Files          29       29              
  Lines        4456     4456              
==========================================
- Hits         4355     4311      -44     
- Misses        101      145      +44

Flag	Coverage Δ
external	`?`
functional	`?`
integration	`?`
unit	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

LecrisUT · 2023-12-04T16:23:28Z

About the codecov issue (Unable to locate build via Github Actions API). Ping me if it happens again, and we can try setting env_vars ¹²

.github/workflows/ci.yml

mahendrapaipuri · 2023-12-04T21:47:25Z

.github/workflows/release.yml

+  release:
+    needs: [ publish ]
+    name: Create release
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          ref: ${{ inputs.ref }}
+      - uses: softprops/action-gh-release@v1
+        with:
+          name: Jupytext ${{ inputs.ref || github.ref_name }}
+          draft: true
+          prerelease: ${{ contains(inputs.ref || github.ref, 'rc') }}
+          generate_release_notes: true


Instead of reinventing the wheel, I would suggest to look into jupyter-releaser. It will work out-of-the-box nicely to publish to both PyPI and NPM.

Sure, open to suggestions on this one, can you propose the change so I can look at what it does and check if it's equivalent? I'm curious if it works with Trusted publishing and how that works when being used with both PyPI and NPM

It does work with Trusted publishing which is very neat. For NPM we still need to use a token as a secret to publish.

I need to read it carefully, because the workflow seems incompatible:
https://github.com/jupyter-server/jupyter_releaser/blob/43d510770f1fe039e133890936416956b927e250/jupyter_releaser/actions/populate_release.py#L42

(This workflow runs on tag release, while the jupyter_releasser creates the new tag) 🤔. Indeed there are good points like the bumpimg of package.json, but then how do you specify the tag?

This is a manual workflow. So, the maintainer will kickstart a release process by prepping a release -> https://jupyter-releaser.readthedocs.io/en/latest/get_started/making_release_from_repo.html#prep-release

And this will produce the artifacts and updated changelog. Once the maintainer is happy with everything, they can go ahead with release -> https://jupyter-releaser.readthedocs.io/en/latest/get_started/making_release_from_repo.html#publish-release

Ok, I think we have two possible workflows here:

run on tag: use the current approach and run pre-release to bump to the next version. I think we can mix individual steps from the releaser as long as we order them appropriately

run on dispatch: use the reusable workflows there

It is a bit weird because of the mixing of dynamic-version via git on the python sideand the static version of npm. Normally I prefer to bump versions after a release in order to clearly communicate the difference in versions (e.g. main vs tag release)

Well, my original suggestion was to replace the current release.yml file with the ones from jupyter-releaser instead of mixing them with current one. But I see few potential blockers adopting jupyter-releaser:

jupyter-releaser expects package.json to be in root of the repo for it to build and release NPM packages.

And NPM package and Python package should have same version for consistency. For example, JupyterLab has few dozens of NPM packages and they all have consistent version with main JupyterLab Python package.

dynamic-version via git on the python sideand the static version of npm

We do not use dynamic version (if you mean like versioneer creating a new version string for each commit). What hatch will do is bump NPM version in package.json and then make sure that Python package will use that same version as package.json. Currently, versions of NPM package and Python package of Jupytext are diverged and hence, it does not work out-of-the-box.

Your suggestion of creating our own workflows based on jupyter-releaser might work. Something like splitting like these workflows for Python and NPM packages separately. But that increases the maintainence burden. As @mwouts is the one who is making releases, it depends on how much complexity he is willing to handle.

Well, my original suggestion was to replace the current release.yml file with the ones from jupyter-releaser instead

Slight nuance because you still need a release.yml with an on:workflow_dispatch. But yeah, what you suggest is about the second approach in my last comment. We just need to have a light wrapper to forward the inputs.

We do not use dynamic version (if you mean like versioneer creating a new version string for each commit).

Ah you're right src/jupytext/version.py is a static file. I confused it with tool.hatch.build.hooks.vcs.version-file. Would the version bumping work on the python side?

Currently, versions of NPM package and Python package of Jupytext are diverged and hence, it does not work out-of-the-box.

Good point. In either case it needs to re-converge the versioning number, unless there are more complicated tag forms.

Your suggestion of creating our own workflows based on jupyter-releaser might work. Something like splitting like these workflows for Python and NPM packages separately.

Indeed, looking back there are quite a few changes necessary. And in either case we need to test these workflows because there seems to be quite a few things to look out for. @mahendrapaipuri can the jupyter-releaser be tested locally on a fork, like uploading to GitHub package repo. And indeed the main question is @mwouts, which one do you prefer: tag push, calling workflow, or manual commits?

Ah you're right src/jupytext/version.py is a static file. I confused it with tool.hatch.build.hooks.vcs.version-file. Would the version bumping work on the python side?

Ah, I thought we are already using hatch to do version bumping. But currenctly version string is set manually I assume. Yes, we can do the bumping using hatch

can the jupyter-releaser be tested locally on a fork, like uploading to GitHub package repo

Yes, there is a check-release workflow that dry runs all the release steps on local PyPI and mock GitHub instance in CI. So, if and when we decide to use jupyter-releaser tools in release workflows, we can adopt a similar solution to test it locally on mock instances.

Thanks @LecrisUT and @mahendrapaipuri for this. I am afraid this is too unfamiliar to me. I am happy with the current process for releasing, i.e. editing the CHANGELOG.md and version files manually, and then adding a tag in the GitHub interface, so if you don't mind I might left this change for later and revert to the previous state of publish.yml for now.

.github/workflows/step_coverage.yml

pyproject.toml

mwouts · 2023-12-07T20:45:53Z

Hi @LecrisUT , thank you for making this PR! I have a busy week but I will certainly look into this over the coming days. Thanks!

Only the workflows that trigger need the concurrency

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

See https://setuptools-scm.readthedocs.io/en/latest/usage/#git-archives

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

mwouts

Sounds great! Thank you for the PR! As mentioned above I might revert the change on release.yml as I am not ready for that one, but other than that I am looking forward to merging this!

mwouts · 2024-01-12T19:06:28Z

.github/workflows/release.yml

+  release:
+    needs: [ publish ]
+    name: Create release
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          ref: ${{ inputs.ref }}
+      - uses: softprops/action-gh-release@v1
+        with:
+          name: Jupytext ${{ inputs.ref || github.ref_name }}
+          draft: true
+          prerelease: ${{ contains(inputs.ref || github.ref, 'rc') }}
+          generate_release_notes: true


Thanks @LecrisUT and @mahendrapaipuri for this. I am afraid this is too unfamiliar to me. I am happy with the current process for releasing, i.e. editing the CHANGELOG.md and version files manually, and then adding a tag in the GitHub interface, so if you don't mind I might left this change for later and revert to the previous state of publish.yml for now.

mwouts · 2024-01-12T19:10:40Z

.github/workflows/step_tests-pip.yml

          - python-version: "3.x"
-            markdown-it-py-version: ""
+            markdown-it-py: false


Testing that Jupytext works without markdown-it-py in the CI seems very hard to achieve! It does not seem to be working on main, but I am not sure it is working here neither. I will have a look.

Cf. https://github.com/mwouts/jupytext/actions/runs/7506308274/job/20437528721 which does not run the 'uninstall markdown-it-py' step:

mwouts · 2024-01-12T19:12:10Z

.github/workflows/step_tests-pip.yml

      - name: Uninstall markdown-it-py
        # Markdown-it-py is a dependency of Jupytext,
        # but Jupytext should still work if it is not installed
-        if: ${{ matrix.markdown-it-py-version == '' }}
+        if: ${{ matrix.markdown-it-py == 'false' }}


Maybe this should be == false? Maybe I'll switch to using a flag like no_kernel or quarto which works and also has the advantage of being more explicit.

Yes you're right. Github workflow is annoying like that where sometimes the variable is converted to a string, sometimes it's the original json type. In the case of matrix it's the latter.

FYI I have added an additional no_markdown-it-py (false/true) at #1204

mwouts · 2024-01-12T19:15:18Z

.github/workflows/step_tests-pip.yml

          - python-version: "3.x"
-            pip-flags: "--pre --upgrade --upgrade-strategy eager"
+            dependency_type: "pre"


Thanks for this! At the moment the pre-pip tests are not passing (an issue with the pytest release candidate). Do you think there is a way to mark this step as experimental, i.e. get a red box for that particular one but still get a pass globally?

Yes, I've recently designed a workflow like that, I'll add it here too: https://github.com/LecrisUT/CMake-Template/blob/a4f73dbd53d8cb98e6325296c2905b2ed40dcaa3/.github/workflows/step_test.yaml#L46

Interesting! I am not familiar with the inputs yet but yes this seems to be the way to go. I've added the experimental flag in this PR, on top of yours: #1204

mwouts · 2024-01-12T19:16:24Z

pyproject.toml

-    "mdit_py_plugins",
-    "markdown-it-py>=1.0.0",
+    "mdit-py-plugins",
+    "markdown-it-py>=2.0",


I am planning to release this PR in jupytext_1.16.1 so I'd rather let this change of dependencies on markdown-it-py for jupytext_1.17.0.

Ok, let me add the CI to run for 1.0 in this case then revert the commit after that

Great idea! Thanks

LecrisUT force-pushed the ci/organize branch 8 times, most recently from 858b829 to d4a633d Compare December 4, 2023 16:06

LecrisUT mentioned this pull request Dec 4, 2023

All the commits are marked with a little red cross #1172

Closed

mahendrapaipuri reviewed Dec 4, 2023

View reviewed changes

LecrisUT force-pushed the ci/organize branch from 27a7c98 to 5bfa66d Compare December 5, 2023 10:11

mahendrapaipuri reviewed Dec 5, 2023

View reviewed changes

.github/workflows/step_coverage.yml Outdated Show resolved Hide resolved

LecrisUT force-pushed the ci/organize branch 2 times, most recently from 0327f0b to 7556316 Compare December 5, 2023 11:11

LecrisUT marked this pull request as ready for review December 5, 2023 11:13

LecrisUT force-pushed the ci/organize branch 2 times, most recently from e3d6c25 to 903e871 Compare December 5, 2023 13:21

LecrisUT mentioned this pull request Dec 5, 2023

ci: Configure ruff #1193

Draft

mahendrapaipuri reviewed Dec 5, 2023

View reviewed changes

pyproject.toml Show resolved Hide resolved

mwouts added this to the 1.17.0 milestone Jan 7, 2024

LecrisUT added 6 commits January 12, 2024 18:59

Remove concurrency

635c3b6

Only the workflows that trigger need the concurrency

Update action versions

1ed9ab2

Reorganize test dependencies

5e0bc6f

Harden the workflows

773b977

Update python-version defaults

1f1d580

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

Remove explicit markdown-it-py

c3bba69

LecrisUT added 15 commits January 12, 2024 18:59

Simplify release CI

afd6891

Improve CI display

6db16ba

Add pass action

332fc1a

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

Simplify step_build

57176f7

Remove the sdist.exclude

5145497

Remove jupyterlab upper-bound

bea7f2c

Add missing .git_archival.txt

4927d5f

See https://setuptools-scm.readthedocs.io/en/latest/usage/#git-archives

Configure fail-fast option

a2f8106

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

Bump minimum markdown-it-py

e5dcfa6

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

fix markdown-it-py-version CI

b43b625

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

Package name convention

dc3b9fd

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

Configure release CD

0ecd51f

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

Run pytest in parallel

269d225

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

Reconfigure codecov

113d2c2

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

Conditionally upload code coverage

1870d19

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>

mwouts force-pushed the ci/organize branch from 0aa50e0 to 1870d19 Compare January 12, 2024 18:59

mwouts reviewed Jan 12, 2024

View reviewed changes

mwouts mentioned this pull request Jan 12, 2024

Update the CI #1204

Merged

mwouts modified the milestones: 1.17.0, 1.16.1 Jan 12, 2024

mwouts mentioned this pull request Jan 12, 2024

Ensure we set jupytext metadata to model #1197

Merged

mwouts merged commit 9046930 into mwouts:main Jan 13, 2024
24 of 27 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: Minor reorganizations #1190

ci: Minor reorganizations #1190

LecrisUT commented Dec 4, 2023 •

edited

Loading

github-actions bot commented Dec 4, 2023

codecov bot commented Dec 4, 2023 •

edited

Loading

LecrisUT commented Dec 4, 2023

mahendrapaipuri Dec 4, 2023

LecrisUT Dec 4, 2023

mahendrapaipuri Dec 4, 2023

LecrisUT Dec 4, 2023

mahendrapaipuri Dec 4, 2023

LecrisUT Dec 4, 2023 •

edited

Loading

mahendrapaipuri Dec 5, 2023

LecrisUT Dec 5, 2023

mahendrapaipuri Dec 5, 2023

mwouts Jan 12, 2024 •

edited

Loading

mwouts commented Dec 7, 2023

mwouts left a comment

mwouts Jan 12, 2024 •

edited

Loading

mwouts Jan 12, 2024

mwouts Jan 12, 2024

LecrisUT Jan 12, 2024

mwouts Jan 12, 2024

mwouts Jan 12, 2024

LecrisUT Jan 12, 2024

mwouts Jan 12, 2024

mwouts Jan 12, 2024

LecrisUT Jan 12, 2024

mwouts Jan 12, 2024

ci: Minor reorganizations #1190

ci: Minor reorganizations #1190

Conversation

LecrisUT commented Dec 4, 2023 • edited Loading

github-actions bot commented Dec 4, 2023

codecov bot commented Dec 4, 2023 • edited Loading

Codecov Report

LecrisUT commented Dec 4, 2023

Footnotes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LecrisUT Dec 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwouts Jan 12, 2024 • edited Loading

Choose a reason for hiding this comment

mwouts commented Dec 7, 2023

mwouts left a comment

Choose a reason for hiding this comment

mwouts Jan 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LecrisUT commented Dec 4, 2023 •

edited

Loading

codecov bot commented Dec 4, 2023 •

edited

Loading

LecrisUT Dec 4, 2023 •

edited

Loading

mwouts Jan 12, 2024 •

edited

Loading

mwouts Jan 12, 2024 •

edited

Loading