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.

Sign up for GitHub

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: fewer triggers #457

Merged
merged 24 commits into from
Jul 6, 2023
Merged

CI: fewer triggers #457

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
ded6758
avoid test build on draft PR
DaniBodor Jun 27, 2023
6885707
coveralls without the if
DaniBodor Jun 27, 2023
57df351
skip all tests on draft PR
DaniBodor Jun 27, 2023
b04236e
limit actions based on path/filetype
DaniBodor Jun 27, 2023
acb005f
include mlc config
DaniBodor Jun 27, 2023
84bef1b
include actions and setug as trigger for build
DaniBodor Jun 27, 2023
a6e53cd
replace single for double quotation marks
DaniBodor Jun 27, 2023
98ceb05
try out change to test_trainer
DaniBodor Jun 27, 2023
a8523f3
remove ignore paths
DaniBodor Jun 27, 2023
5dc021b
remove mlc config
DaniBodor Jun 27, 2023
e77a9ae
try without paths
DaniBodor Jun 27, 2023
dce6d4c
spaces?
DaniBodor Jun 27, 2023
7b61248
test
DaniBodor Jun 27, 2023
7b16079
readd paths on push
DaniBodor Jun 27, 2023
d4e1c3a
dont specify folders
DaniBodor Jun 27, 2023
742d0b4
dont ignore paths
DaniBodor Jun 27, 2023
a33cbda
can only use paths XOR ignore-paths (not both)
DaniBodor Jun 27, 2023
922cff0
only run build on 3.9
DaniBodor Jul 4, 2023
4150f6a
ignore paths rather than include paths
DaniBodor Jul 6, 2023
bf222fc
run coveralls on python 3.10
DaniBodor Jul 6, 2023
14b8ee7
make coveralls action equivalent to build action
DaniBodor Jul 6, 2023
f456912
Revert "make coveralls action equivalent to build action"
DaniBodor Jul 6, 2023
05170fb
xRevert "run coveralls on python 3.10"
DaniBodor Jul 6, 2023
3b24f92
re-add 3.10 build
DaniBodor Jul 6, 2023
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
13 changes: 0 additions & 13 deletions .github/workflows/.mlc-config.json
View file
Open in desktop

This file was deleted.

34 changes: 29 additions & 5 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,49 @@
name: build

on:
# Trigger analysis when pushing in main or pull requests, and when creating
# a pull request.
push:
paths:
# specific folder locations
- "deeprankcore/**"
- "tests/**"
# exclude filetypes
- "!**.md"
- "!**.rst"
- "!**.ipynb"
# self
- ".github/workflows/build.yml"
# setup
- ".github/actions/install-python-and-package/action.yml"
- "setup.cfg"
branches:
- main
pull_request:
types: [opened, synchronize, reopened]

types: [opened, synchronize, reopened, ready_for_review]
paths:
# specific folder locations
- "deeprankcore/**"
- "tests/**"
# exclude filetypes
- "!**.md"
- "!**.rst"
- "!**.ipynb"
# self
- ".github/workflows/build.yml"
# setup
- ".github/actions/install-python-and-package/action.yml"
- "setup.cfg"

jobs:
build:
if: github.event.pull_request.draft == false
name: Build for (${{ matrix.python-version }}, ${{ matrix.os }})
runs-on: ${{ matrix.os }}

strategy:
fail-fast: false
matrix:
os: ['ubuntu-latest']
python-version: ['3.9', '3.10']
python-version: ['3.9']
Copy link
Collaborator

@gcroci2 gcroci2 Jul 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

If we say that the package work with python 3.9 and 3.10, this needs to be tested and verified with the CI. Either we decide that we don't support python 3.9 anymore (because in case I'd go for the newer python and not for the older version), or we keep both versions in the CI.

I would keep both, I don't think it's critical in our case (testing on the two latest releases of python is pretty common and we've already removed python 3.8 at the beginning of the project). Also it could be that some unix based OS still need python 3.9 for let pytorch work. At the same time we want to guarantee that the package works with the latest release of python (for example because it's faster than 3.9)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Newest python is actually 3.11 :). But let's stick to 3.9 and 3.10 for now, I think.
We could do 3.9 for the normal build and 3.10 for the coveralls build. There is currently 1 command present in the normal build that isn't in coveralls, I can add that one to the coveralls to basically make it an extension of normal build.

Copy link
Collaborator

@gcroci2 gcroci2 Jul 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Oh and it was released in October, so many months ago. Then in the early future we should think about adding 3.11 as well, and at that point we can evaluate how critical is to remove the 3.9 version.

For the building, I get that coveralls does the build itself so you want to test 3.9 with the build file and 3.10 with the coveralls file, because both builds are tested this way. But this is not the cleaner way of doing it, because these yml files are thought to have well separated functions and they originate the badges that we show in the readme:
image

The build should be verified separately from coveralls, which should be run only if the build jobs go well. There are ways or creating interdependent yml files and avoiding running stuff multiple times, but I didn't want to spend much time on implementing more elegant solutions and I sticked to the basic one. If you're interested of course go for it, and we can have a chat for looking together where to find such info, but otherwise I'd prefer to use the build file to actually verify the builds that we want to guarantee (3.9 and 3.10 so far), and then use one of the two versions for running coveralls.


steps:
- uses: actions/checkout@v3
Expand Down
29 changes: 25 additions & 4 deletions .github/workflows/coveralls.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,41 @@ name: coveralls

on:
push:
paths:
# specific folder locations
- "deeprankcore/**"
- "tests/**"
# exclude filetypes
- "!**.md"
- "!**.rst"
- "!**.ipynb"
# self
- ".github/workflows/coveralls.yml"
branches:
- main
- main
pull_request:
types: [opened, synchronize, reopened]
types: [opened, synchronize, reopened, ready_for_review]
paths:
# specific folder locations
- "deeprankcore/**"
- "tests/**"
# exclude filetypes
- "!**.md"
- "!**.rst"
- "!**.ipynb"
# self
- ".github/workflows/coveralls.yml"

jobs:
build:
if: github.event.pull_request.draft == false
name: Coveralls
runs-on: ${{ matrix.os }}

strategy:
fail-fast: false
matrix:
os: ['ubuntu-latest']
os: ["ubuntu-latest"]
python-version: [3.9]

steps:
Expand All @@ -26,6 +47,6 @@ jobs:
- name: Coveralls
env:
GITHUB_TOKEN: ${{ secrets.github_token }}
COVERALLS_FLAG_NAME: python-${{ matrix.version }}
COVERALLS_FLAG_NAME: python-${{ matrix.os }}
run: |
coveralls --service=github
3 changes: 2 additions & 1 deletion .github/workflows/fair-software.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ on:
branches:
- main
pull_request:
types: [opened, synchronize, reopened]
types: [opened, synchronize, reopened, ready_for_review]

jobs:
verify:
if: github.event.pull_request.draft == false
name: "fair-software"
runs-on: ubuntu-latest
steps:
Expand Down
27 changes: 24 additions & 3 deletions .github/workflows/linting.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,41 @@ name: linting

on:
push:
paths:
# specific folder locations
- "deeprankcore/**"
- "tests/**"
# exclude filetypes
- "!**.md"
- "!**.rst"
- "!**.ipynb"
# self
- ".github/workflows/linting.yml"
branches:
- main
- main
pull_request:
types: [opened, synchronize, reopened]
types: [opened, synchronize, reopened, ready_for_review]
paths:
# specific folder locations
- "deeprankcore/**"
- "tests/**"
# exclude filetypes
- "!**.md"
- "!**.rst"
- "!**.ipynb"
# self
- ".github/workflows/linting.yml"

jobs:
build:
if: github.event.pull_request.draft == false
name: Linting build
runs-on: ${{ matrix.os }}

strategy:
fail-fast: false
matrix:
os: ['ubuntu-latest']
os: ["ubuntu-latest"]
python-version: [3.9]

steps:
Expand Down
17 changes: 14 additions & 3 deletions .github/workflows/markdown-link-check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,24 @@ on:
push:
branches:
- main
paths:
# filetypes
- "**.md"
- "**.rst"
# self
- ".github/workflows/markdown-link-check.yml"
pull_request:
branches:
- main
types: [opened, synchronize, reopened, ready_for_review]
paths:
# filetypes
- "**.md"
- "**.rst"
# self
- ".github/workflows/markdown-link-check.yml"

jobs:

markdown-link-check:
if: github.event.pull_request.draft == false
name: Check markdown links
runs-on: ubuntu-latest
steps:
Expand Down
1 change: 0 additions & 1 deletion tests/test_trainer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,5 @@ def test_invalid_no_cuda_available(self):




if __name__ == "__main__":
unittest.main()