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

Set proton version in CI to 0.39.0 #1115

Merged

Conversation

ganeshmurthy
Copy link
Contributor

No description provided.

@ganeshmurthy ganeshmurthy requested review from jiridanek and kgiusti June 6, 2023 13:17
@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

Merging #1115 (3d7e573) into main (6055594) will increase coverage by 11.41%.
The diff coverage is 75.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1115       +/-   ##
===========================================
+ Coverage   66.50%   77.91%   +11.41%     
===========================================
  Files         318      238       -80     
  Lines       64589    60690     -3899     
  Branches        0     5575     +5575     
===========================================
+ Hits        42954    47289     +4335     
+ Misses      21635    10773    -10862     
- Partials        0     2628     +2628     
Flag Coverage Δ
pysystemtests 87.34% <ø> (+20.84%) ⬆️
pyunittests 54.51% <ø> (?)
systemtests 71.69% <75.00%> (?)
unittests 27.21% <30.55%> (?)

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

Components Coverage Δ
calculator 30.82% <30.55%> (∅)
systemtests 78.52% <75.00%> (+12.02%) ⬆️

@jiridanek
Copy link
Contributor

@jiridanek
Copy link
Contributor

The failure https://github.com/skupperproject/skupper-router/actions/runs/5189041316/jobs/9353483797?pr=1115#step:8:91

error: Bad exit status from /var/tmp/rpm-tmp.j0ul61 (%build)
    Downloading https://www.apache.org/dist/qpid/proton/0.39.0/qpid-proton-0.39.0.tar.gz to /tmp/skupper-rpms/qpid-proton-0.39.0.tar.gz
    Bad exit status from /var/tmp/rpm-tmp.j0ul61 (%build)

is probably just because the apache mirrors did not have enough time to sync the new release.

@ganeshmurthy
Copy link
Contributor Author

@jiridanek
Copy link
Contributor

Also, this

but I'd be willing to bet it is just revealing some previously existing issue, most likely in tests themselves.

@ganeshmurthy
Copy link
Contributor Author

Also, this

* [ AutoLinkRetryTest.test_auto_link_reattach: AssertionError: 'attaching' != 'failed' #1116](https://github.com/skupperproject/skupper-router/issues/1116)

but I'd be willing to bet it is just revealing some previously existing issue, most likely in tests themselves.

I am spending some time today looking at some intermittent errors, I will look at this one.

@jiridanek
Copy link
Contributor

There is a failure here - https://github.com/skupperproject/skupper-router/actions/runs/5189041316/jobs/9353483797?pr=1115#step:8:92

@ganeshmurthy Uh, I can't read error messages.

The relevant error message part is

CMake Error at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find Python: Found unsuitable version "3.6.8", but required is at
  least "3.8" (found /usr/bin/python3, found components: Interpreter
  Development Development.Module Development.Embed)

and that comes from new Proton wanting newer Python. We can either use python3.9 in the RPM package, or build the rpm package for RHEL 9. There is PR for the first solution which would need to be rebased and merged

@ganeshmurthy
Copy link
Contributor Author

There is a failure here - https://github.com/skupperproject/skupper-router/actions/runs/5189041316/jobs/9353483797?pr=1115#step:8:92

@ganeshmurthy Uh, I can't read error messages.

The relevant error message part is

CMake Error at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find Python: Found unsuitable version "3.6.8", but required is at
  least "3.8" (found /usr/bin/python3, found components: Interpreter
  Development Development.Module Development.Embed)

and that comes from new Proton wanting newer Python. We can either use python3.9 in the RPM package, or build the rpm package for RHEL 9. There is PR for the first solution which would need to be rebased and merged

* [Issue #910, #48, #102 - Require Python 3.9 in RPM package #398](https://github.com/skupperproject/skupper-router/pull/398)

I feel like going the RHEL 9 route (since this is upstream) is the right thing to do, what do you think ?

@jiridanek
Copy link
Contributor

jiridanek commented Jun 6, 2023

I feel like going the RHEL 9 route (since this is upstream) is the right thing to do, what do you think?

There's nothing wrong in having a RPM that works on RHEL 8 (and variants) as well. It is a bit more work, but the work will be useful to anyone not yet on RHEL 9.

Trying to solve RHEL 7 here would be out of place, I agree. RHEL 8 is fine.

edit: so if @mcressman is going to be solving RHEL 7 as well, we won't help him much here, because RHEL 7 would require softwarecollections... I wouldn't want this in upstream. And in that case it might make sense to jump to RHEL 9 straight away.

If you look at the RPM PR, you'd see that the change being made also removes the schizophrenic situation where router uses self-compiled proton while router tools use packaged qpid-proton-python. That's another good reason (IMO) to go with the PR.

@ganeshmurthy
Copy link
Contributor Author

And in that case it might make sense to jump to RHEL 9 straight away.

The other reason I prefer rhel 9 is because this is upstream and we want to generate rpms for the latest rhel.
I am thinking of merging this PR and you can go ahead and change the rpm generation PR to use rhel 9 ?

@jiridanek
Copy link
Contributor

The other reason I prefer rhel 9 is because this is upstream and we want to generate rpms for the latest rhel.

Rhel is an exception from the "stay on head" upstream rule, I think.

I am thinking of merging this PR and you can go ahead and change the rpm generation PR to use rhel 9 ?

Fine with me

@jiridanek
Copy link
Contributor

And in that case it might make sense to jump to RHEL 9 straight away.

The other reason I prefer rhel 9 is because this is upstream and we want to generate rpms for the latest rhel. I am thinking of merging this PR and you can go ahead and change the rpm generation PR to use rhel 9 ?

kgiusti pushed a commit to kgiusti/skupper-router that referenced this pull request Jul 17, 2023
kgiusti pushed a commit that referenced this pull request Jul 20, 2023
jiridanek pushed a commit to jiridanek/skupper-router that referenced this pull request Sep 19, 2023
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.

2 participants