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

ci: Minor reorganizations #1190

Merged
merged 21 commits into from
Jan 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .git_archival.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
node: $Format:%H$
node-date: $Format:%cI$
describe-name: $Format:%(describe:tags=true,match=*[0-9]*)$
ref-names: $Format:%D$
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
World\ population.ipynb linguist-documentation
.git_archival.txt export-subst
5 changes: 1 addition & 4 deletions .github/codecov.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
codecov:
notify:
after_n_builds: 16

comment:
after_n_builds: 16
wait_for_ci: true

coverage:
status:
Expand Down
45 changes: 30 additions & 15 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,16 @@ on:
push:
paths-ignore:
- "CHANGELOG.md"
branches: [main]
branches: [ main ]
pull_request:
branches: [main]
branches: [ main ]
schedule:
- cron: "0 11 * * 4"

permissions:
# All nested workflows will inherit these permissions and so no need to declare
# in each step file
contents: read
# Cannot use it in codeql nested workflow without declaring it on
# top level workflow
# Ref: https://docs.github.com/en/actions/using-workflows/reusing-workflows#access-and-permissions
security-events: write

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
Expand All @@ -33,28 +29,47 @@ jobs:
pre-commit:
uses: ./.github/workflows/step_pre-commit.yml

codeql:
needs: [pre-commit]
static-analysis:
needs: [ pre-commit ]
uses: ./.github/workflows/step_static-analysis.yml
permissions:
contents: read
security-events: write

test-pip:
needs: [codeql]
needs: [ pre-commit ]
uses: ./.github/workflows/step_tests-pip.yml
with:
coverage: ${{ github.event_name != 'schedule' }}

test-unit-functional-integration:
needs: [codeql]
uses: ./.github/workflows/step_test_unit_functional.yml
coverage:
needs: [ test-pip ]
uses: ./.github/workflows/step_coverage.yml
if: github.event_name != 'schedule'

test-conda:
needs: [codeql]
needs: [ test-pip ]
uses: ./.github/workflows/step_tests-conda.yml
with:
coverage: ${{ github.event_name != 'schedule' }}

test-ui:
needs: [codeql]
needs: [ test-pip ]
uses: ./.github/workflows/step_tests-ui.yml

build:
needs: [test-pip, test-conda, test-unit-functional-integration, test-ui]
needs: [ test-pip, test-conda, test-ui ]
uses: ./.github/workflows/step_build.yml
with:
upload: ${{ inputs.upload-build-artifacts || false }}

pass:
name: Pass
needs: [ pre-commit, static-analysis, test-pip, coverage, test-conda, test-ui, build ]
runs-on: ubuntu-latest
steps:
- uses: re-actors/alls-green@release/v1
with:
jobs: ${{ toJSON(needs) }}
allowed-skips: coverage
if: always()
2 changes: 1 addition & 1 deletion .github/workflows/comment-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
name: Comment PR
steps:
- name: Checkout
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Comment PR
uses: thollander/actions-comment-pull-request@v2
Expand Down
61 changes: 0 additions & 61 deletions .github/workflows/publish.yml

This file was deleted.

96 changes: 96 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
name: Release
on:
push:
tags:
- "v[0-9]+.[0-9]+.[0-9]+"
- "v[0-9]+.[0-9]+.[0-9]+rc[0-9]+"
- "v[0-9]+.[0-9]+.[0-9]+dev[0-9]+"
workflow_dispatch:
inputs:
skip-tests:
type: boolean
description: Skip
default: false
ref:
type: string
description: Tag to release
required: true

permissions:
contents: read

jobs:
pre-commit:
uses: ./.github/workflows/step_pre-commit.yml
if: "! inputs.skip-tests"

test-pip:
needs: [ pre-commit ]
uses: ./.github/workflows/step_tests-pip.yml
with:
coverage: false
if: "! inputs.skip-tests"

test-conda:
needs: [ test-pip ]
uses: ./.github/workflows/step_tests-conda.yml
with:
coverage: false
if: "! inputs.skip-tests"

test-ui:
needs: [ test-pip ]
uses: ./.github/workflows/step_tests-ui.yml
if: "! inputs.skip-tests"

test-status:
needs: [ pre-commit, test-pip, test-conda, test-ui ]
runs-on: ubuntu-latest
steps:
- uses: re-actors/alls-green@release/v1
with:
allowed-skips: pre-commit, test-pip, test-conda, test-ui
jobs: ${{ toJSON(needs) }}
if: always()

build:
needs: [ test-status ]
uses: ./.github/workflows/step_build.yml
with:
upload: true
ref: ${{ inputs.ref }}

publish:
needs: [ build ]
runs-on: ubuntu-latest

environment:
name: pypi
url: https://pypi.org/p/jupytext

permissions:
contents: read
id-token: write

steps:
- uses: actions/download-artifact@v3
with:
name: dist
path: dist
- name: Publish
uses: pypa/gh-action-pypi-publish@release/v1

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
Comment on lines +83 to +96
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@LecrisUT LecrisUT Dec 4, 2023

Choose a reason for hiding this comment

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

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)

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Owner

@mwouts mwouts Jan 12, 2024

Choose a reason for hiding this comment

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

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.

41 changes: 12 additions & 29 deletions .github/workflows/step_build.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: build
run-name: Build test
run-name: Build package

on:
workflow_call:
Expand All @@ -9,46 +9,29 @@ on:
required: false
default: false
description: Upload build artifacts
ref:
type: string
description: Tag to build

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-build
cancel-in-progress: true
permissions:
contents: read

jobs:
build:
runs-on: ubuntu-latest
steps:
- name: Checkout source
uses: actions/checkout@v3
uses: actions/checkout@v4
with:
ref: ${{ inputs.ref }}

- name: Base Setup
uses: jupyterlab/maintainer-tools/.github/actions/base-setup@v1
with:
python_version: "3.x"

- name: Build package
run: |
python -m pip install build wheel

# NOTE: These builds and verifications of the builds can be done more
# robustly with jupyter-releaser.
#
# Removed the check on size of package as we are distributing tests/ with
# sdist now and they are around 8MB. Seems like original check was to make
# sure we are not distributing node_modules and we are quite safe with that
# with hatch build system.
#
# Build jupytext package
python -m build

# Build lab extension(s)
npm pack --pack-destination dist jupyterlab/packages/*

# Check that the lab is there
if (($(tar -tf dist/*.tar.gz | grep jupyterlab/jupyterlab_jupytext/labextension/package.json$ | wc -l)==0)); then echo "Missing jupyterlab_jupytext" && exit 1; fi

# Install package and extension
python -m pip install dist/*.tar.gz

echo "Install went OK"
run: hatch build

- name: Archive build artifacts
uses: actions/upload-artifact@v3
Expand Down
Original file line number Diff line number Diff line change
@@ -1,41 +1,40 @@
name: test-categories
run-name: Run Unit/Functional/Integration and External tests using Pip
name: coverage
run-name: Check coverage

on:
workflow_call:

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-test_classification
cancel-in-progress: true
permissions:
contents: read

jobs:
test-pip:
continue-on-error: ${{ matrix.experimental || false }}
coverage:
name: >
${{ matrix.coverage }}
runs-on: ubuntu-latest

strategy:
fail-fast: false
matrix:
python-version: [ "3.11" ]
coverage: [unit, functional, integration, external]

steps:
- name: Checkout
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Base Setup
uses: jupyterlab/maintainer-tools/.github/actions/base-setup@v1
with:
python_version: ${{ matrix.python-version }}
python_version: 3.x

- name: Install from source
run: HATCH_BUILD_HOOKS_ENABLE=false python -m pip install -e '.[test-cov,test-${{ matrix.coverage }}]' markdown-it-py~=3.0
run: HATCH_BUILD_HOOKS_ENABLE=false python -m pip install -e '.[test-cov,test-${{ matrix.coverage }}]'

- name: Install a Jupyter Kernel
run: python -m ipykernel install --name python_kernel --user

- name: Run the tests
run: pytest tests/${{ matrix.coverage }} --cov
run: pytest tests/${{ matrix.coverage }} -n logical --cov --cov-report=xml

- name: Upload the coverage
uses: codecov/codecov-action@v3
Expand Down
Loading
Loading