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

Small fixes for CICD #414

Merged
merged 3 commits into from
Feb 18, 2025
Merged

Small fixes for CICD #414

merged 3 commits into from
Feb 18, 2025

Conversation

dkropachev
Copy link
Collaborator

@dkropachev dkropachev commented Jan 26, 2025

  1. Make it not to trigger building CICD if no code changed
  2. Make all workflows manually trigger-able
  3. Remove build-pre-release.yml

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • I added relevant tests for new features and bug fixes.
  • All commits compile, pass static checks and pass test.
  • PR description sums up the changes and reasons why they should be introduced.
  • I have provided docstrings for the public items that I want to introduce.
  • I have adjusted the documentation in ./docs/source/.
  • I added appropriate Fixes: annotations to PR description.

@dkropachev
Copy link
Collaborator Author

@fruch , I want to remove build-pre-release.yml because it is not achieving anything, just running extra laps.

Amongst all drivers python-driver pipeline is the most expensive one, I want to reduce amount of resources it consumes.

@dkropachev dkropachev marked this pull request as ready for review January 26, 2025 23:03
@fruch
Copy link

fruch commented Jan 27, 2025

@fruch , I want to remove build-pre-release.yml because it is not achieving anything, just running extra laps.

Amongst all drivers python-driver pipeline is the most expensive one, I want to reduce amount of resources it consumes.

it's idea was to start testing before there's a release, if it doesn't do nothing now, it's not really expensive
I can go, but I think still we need todo the needed work before the python release is out, and not when avi want to move to newer fedora...

@dkropachev
Copy link
Collaborator Author

@fruch , I want to remove build-pre-release.yml because it is not achieving anything, just running extra laps.
Amongst all drivers python-driver pipeline is the most expensive one, I want to reduce amount of resources it consumes.

it's idea was to start testing before there's a release, if it doesn't do nothing now, it's not really expensive I can go, but I think still we need todo the needed work before the python release is out, and not when avi want to move to newer fedora...

With another distro, you never know, another libc, another environment, python has too many dependancies, unless you test it, you never know.

But, don't we do run tests in Build and upload to PyPi ?
We run it for every PR, cibuildwheel builds wheels, installs them and runs tests on them.
So, we do that, only problem with case avi, is that he used fedora which we did not test, and extension error where ignored.

@fruch
Copy link

fruch commented Jan 27, 2025

@fruch , I want to remove build-pre-release.yml because it is not achieving anything, just running extra laps.
Amongst all drivers python-driver pipeline is the most expensive one, I want to reduce amount of resources it consumes.

it's idea was to start testing before there's a release, if it doesn't do nothing now, it's not really expensive I can go, but I think still we need todo the needed work before the python release is out, and not when avi want to move to newer fedora...

With another distro, you never know, another libc, another environment, python has too many dependancies, unless you test it, you never know.

But, don't we do run tests in Build and upload to PyPi ? We run it for every PR, cibuildwheel builds wheels, installs them and runs tests on them. So, we do that, only problem with case avi, is that he used fedora which we did not test, and extension error where ignored.

we didn't even build 3.13 yet, when he built.

and now we should have 3.14 (https://www.python.org/downloads/release/python-3140a1/) almost pre-release.
and I don't think we should wait until the formal release of it, to try it out.

@dkropachev
Copy link
Collaborator Author

@fruch , I want to remove build-pre-release.yml because it is not achieving anything, just running extra laps.
Amongst all drivers python-driver pipeline is the most expensive one, I want to reduce amount of resources it consumes.

it's idea was to start testing before there's a release, if it doesn't do nothing now, it's not really expensive I can go, but I think still we need todo the needed work before the python release is out, and not when avi want to move to newer fedora...

With another distro, you never know, another libc, another environment, python has too many dependancies, unless you test it, you never know.
But, don't we do run tests in Build and upload to PyPi ? We run it for every PR, cibuildwheel builds wheels, installs them and runs tests on them. So, we do that, only problem with case avi, is that he used fedora which we did not test, and extension error where ignored.

we didn't even build 3.13 yet, when he built.

and now we should have 3.14 (https://www.python.org/downloads/release/python-3140a1/) almost pre-release. and I don't think we should wait until the formal release of it, to try it out.

Oh, you mean that you want to have a github workflow to enable testing on upcoming python version ?

@fruch
Copy link

fruch commented Jan 27, 2025

@fruch , I want to remove build-pre-release.yml because it is not achieving anything, just running extra laps.
Amongst all drivers python-driver pipeline is the most expensive one, I want to reduce amount of resources it consumes.

it's idea was to start testing before there's a release, if it doesn't do nothing now, it's not really expensive I can go, but I think still we need todo the needed work before the python release is out, and not when avi want to move to newer fedora...

With another distro, you never know, another libc, another environment, python has too many dependancies, unless you test it, you never know.
But, don't we do run tests in Build and upload to PyPi ? We run it for every PR, cibuildwheel builds wheels, installs them and runs tests on them. So, we do that, only problem with case avi, is that he used fedora which we did not test, and extension error where ignored.

we didn't even build 3.13 yet, when he built.

and now we should have 3.14 (https://www.python.org/downloads/release/python-3140a1/) almost pre-release. and I don't think we should wait until the formal release of it, to try it out.

Oh, you mean that you want to have a github workflow to enable testing on upcoming python version ?

Yes, that's what this flow was for I created it for python 3.13 before it was released

@Lorak-mmk
Copy link

Apart from the removal of build-prerelease it looks fine.
The removal I'm not sure about. After other PR is merged (the one that fails the build if the extensions fail to build) this workflow should start to become useful.

@dkropachev
Copy link
Collaborator Author

@fruch , I want to remove build-pre-release.yml because it is not achieving anything, just running extra laps.
Amongst all drivers python-driver pipeline is the most expensive one, I want to reduce amount of resources it consumes.

it's idea was to start testing before there's a release, if it doesn't do nothing now, it's not really expensive I can go, but I think still we need todo the needed work before the python release is out, and not when avi want to move to newer fedora...

With another distro, you never know, another libc, another environment, python has too many dependancies, unless you test it, you never know.
But, don't we do run tests in Build and upload to PyPi ? We run it for every PR, cibuildwheel builds wheels, installs them and runs tests on them. So, we do that, only problem with case avi, is that he used fedora which we did not test, and extension error where ignored.

we didn't even build 3.13 yet, when he built.

and now we should have 3.14 (https://www.python.org/downloads/release/python-3140a1/) almost pre-release. and I don't think we should wait until the formal release of it, to try it out.

Let's make it manually triggerable and don't run it in PRs no to polute statuses and later make it pull current pre-prelease and run on it, WDYT ?

@fruch
Copy link

fruch commented Jan 28, 2025

@fruch , I want to remove build-pre-release.yml because it is not achieving anything, just running extra laps.
Amongst all drivers python-driver pipeline is the most expensive one, I want to reduce amount of resources it consumes.

it's idea was to start testing before there's a release, if it doesn't do nothing now, it's not really expensive I can go, but I think still we need todo the needed work before the python release is out, and not when avi want to move to newer fedora...

With another distro, you never know, another libc, another environment, python has too many dependancies, unless you test it, you never know.
But, don't we do run tests in Build and upload to PyPi ? We run it for every PR, cibuildwheel builds wheels, installs them and runs tests on them. So, we do that, only problem with case avi, is that he used fedora which we did not test, and extension error where ignored.

we didn't even build 3.13 yet, when he built.

and now we should have 3.14 (https://www.python.org/downloads/release/python-3140a1/) almost pre-release. and I don't think we should wait until the formal release of it, to try it out.

Let's make it manually triggerable and don't run it in PRs no to polute statuses and later make it pull current pre-prelease and run on it, WDYT ?

As I said before, If we allocate time for testing it, I'm o.k. with ditching the pipeline. one can test it locally even, but if we don't, we for sure are gonna hit by it

Also the other builds, I think we can remove some of the options, and put them under the label, for example windows / Mac, or aarch64, but before release we need a swipe on those, other release would fail

@dkropachev dkropachev force-pushed the dk/cicd-small-fixes branch 10 times, most recently from ba5e2ce to 576c1ca Compare February 18, 2025 11:33
@dkropachev
Copy link
Collaborator Author

Apart from the removal of build-prerelease it looks fine. The removal I'm not sure about. After other PR is merged (the one that fails the build if the extensions fail to build) this workflow should start to become useful.

it is needed only to test comptibility with next release before it is released, having it in regular CICD will polute PR status with something that have nothing to do with PR.

@Lorak-mmk, @fruch , please take a look again, I have made build and push workflow reusable and reused it for both regular build and pre-releases.

1. Make it reuse `lib-build-and-push`
2. Make it manually triggerable only

This workflow is checking if driver works with next pre-release python version.
We don't want to have them in PRs, if it fails it does not mean that PR
is bad.

It would be better to manually trigger it for now, and later we will make it
check on existing pre-release versions and run on them once a month.
@dkropachev dkropachev merged commit 41e86cb into master Feb 18, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants