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

Sets the support level to python 3.6 or above #88

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

jacomago
Copy link
Contributor

@jacomago jacomago commented May 8, 2024

Removes from the server support for python 3.2 and below
Sets the suggested python version to 3.6 or above

@jacomago
Copy link
Contributor Author

jacomago commented May 8, 2024

As support is being removed more and more for lower python versions. I thought it would make sense to make this change. I expect a lot of sites use a docker image for recceiver anyway, but I will leave this PR unmerged for some time.

@jacomago jacomago self-assigned this May 8, 2024
@jeonghanlee
Copy link
Contributor

jeonghanlee commented May 8, 2024

@jacomago could you collect all user OS requirements? That changes are okay for the ALS-U, since we officially support Debian 12 and Rocky 9.2. Both OSs have the Python 3.11. However, what about other facilities?

I am still thinking these changes are a bit early.

Why not we keep the old codes as it is, and add new version code together? Or do you have any specific reasons regarding security concerns?

@ralphlange
Copy link

I agree with Han.

Don't underestimate user annoyance when doing your cost benefit analysis.

RHEL is widely used and pretty conservative. To me, the standard Python version on RHEL 8 would look like a good marker for what people might want to use.

@tynanford
Copy link
Contributor

RHEL 8 ships with python 3.6.8 and I agree with Ralph to at least support that OS. Looking naively at the PR, it just drops python 2 support? What changes preclude python 3 below 3.7?

RHEL 7 is still around til the end of June and ships with python 2.7 but maybe those users should be focusing more on updating their OS than upgrading recceiver to the latest version in github ;) RHEL 8 is dropping support for python 2.7 at the same time - https://access.redhat.com/solutions/4455511

So dropping python 2 soon would be reasonable to me.

@jeonghanlee
Copy link
Contributor

I also agree with Tynan, we should prepare to drop python 2 soon.

@shroffk
Copy link
Collaborator

shroffk commented May 10, 2024

To sum up

  • We should drop python 2 support.
  • @jacomago are there any changes needed to keep the minimum support to 3.6. Like @tynanford mentioned, I think there are some changes which need at least 3.2 and the rest are simply getting rid of the switch added for supporting both version 2 and 3.
  • since a large number on EPICS institutions use red hat. We can use that to decide the minimum versions we should support. Currently 3.6 seems like a good target (unless we have a strong justification for features in a newer release).

@jacomago
Copy link
Contributor Author

Perfect. Yes, as @tynanford this actually removes support for 3.2 and below. I've updated the PR description. I'll make a follow up PR to add building down to 3.6.

@jeonghanlee
Copy link
Contributor

jeonghanlee commented May 13, 2024

@jacomago 3.2 and below?

So do we support 3.3.X after this merge? Or 3.6.X?

Can we specify the future support version more clearly?

@jacomago
Copy link
Contributor Author

@jeonghanlee I think its best to say this PR keeps the code working for python >=3.3. But we keep support specified in the pyproject.toml which is set to >=3.6. This makes clear we won't fix bugs for running on python <=3.5.

@tynanford
Copy link
Contributor

I think technically it is >=3.2 like @shroffk mentioned. It would be good to update the README with this PR as well - https://github.com/ChannelFinder/recsync?tab=readme-ov-file#recceiver-usage

@jacomago jacomago force-pushed the python37up branch 2 times, most recently from 2e48ed5 to 84dc8ee Compare May 22, 2024 08:32
@ralphlange
Copy link

Might be nitpicking...
Could you update the commit message to better reflect the change? (Could scare people...)

Removes from the server support for python 3.2 and below
Sets the suggested python version to 3.6 or above
Copy link

sonarcloud bot commented May 22, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@jacomago jacomago changed the title Remove support for python 3.6 and below Sets the support level to python 3.6 or above May 22, 2024
@jacomago
Copy link
Contributor Author

Might be nitpicking... Could you update the commit message to better reflect the change? (Could scare people...)

Updated the commit and the PR descriptions.

@shroffk shroffk merged commit 9333214 into ChannelFinder:master Jun 14, 2024
7 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.

5 participants