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

The unbearable optionality of dependencies and features #48

Open
2 of 3 tasks
jiridanek opened this issue Feb 7, 2022 · 4 comments · Fixed by #397
Open
2 of 3 tasks

The unbearable optionality of dependencies and features #48

jiridanek opened this issue Feb 7, 2022 · 4 comments · Fixed by #397
Assignees
Labels
question Further information is requested

Comments

@jiridanek
Copy link
Contributor

jiridanek commented Feb 7, 2022

Qdrouterd build is very adaptable to the circumstances, switching off features and their tests depending on what's available. This is all well for a generic library, but may not be so great for an end-user application where uniformity can simplify some things greatly.

  • For example, should libwebsockets remain to be optional dependency? (I think not.) What about libnghttp2 (There I am not so sure, but I wouldn't leave it optional either.)
  • What about SKIP_DELETE_HTTP_LISTENER config option? (I'd decide if that should be ON or OFF and then made it that way; even if it requires messing with libwebsocket version, because the CentOS one does not suit, then so be it.)

What about Python version? RHEL8/CentOS Stream 8 ship with versions of Python ranging between 3.6 to 3.9. Is there a reason to use the oldest 3.6 version, which means some tests for TLS (that require 3.7) don't run?

  • Python test dependencies can be collected in a requirements-dev.txt file and always installed. That way tests wouldn't be skipping themselves on their own.
@kgiusti
Copy link
Contributor

kgiusti commented Apr 6, 2022

This is probably best converted to a discussion rather than an issue as it raises several points to... discuss. Issues can then be opened as needed from decisions made.

I agree there's many options that really no longer apply in the skupper-router case, so in general if a feature is required for skupper it should not be optional (HTTP_LISTENER is required, so SKIP_DELETE_HTTP_LISTENER is Not Good, for example). libnghttp2 is a must have as well and should no longer be optional. libwebsockets is a requirement as well.

In most (all?) skupper deployments skupper-router will be containerized so we have the liberty to pick whatever version of python is available in the base image that suites our needs. In the case of Centos8 MHO would be to use 3.7 so we can ensure that the TLS feature is deployed against a version of python that we used to test it in CI.

@jiridanek
Copy link
Contributor Author

@kgiusti I think you've resolved pretty much all the questions. Only thing I am not sure about is this: if we make all system_tests mandatory (not auto-skippable), then the RPM %check will have hard time. So my thinking there is that either some good solution is devised (such as, don't run %check in RPMs, or only run it on Fedora where there are RPM-packaged python dependencies available, or run pip install as part of RPM build, and disable network sandbox for the build to permit that, ...) or the tests remain optional, and we somehow better monitor what got skipped (the Jenkins UI helps with that... as much as I am not a fan of Jenkins)

SKIP_DELETE_HTTP_LISTENER is not good? But we need it, because some libwebsocket versions don't permit socket deletion

LIBWEBSOCKETS_VERSION: Optional[Tuple[int, int, int]] = parse_version("${LIBWEBSOCKETS_VERSION_STRING}")
SKIP_DELETE_HTTP_LISTENER = None if not LIBWEBSOCKETS_VERSION else (4, 0, 0) <= LIBWEBSOCKETS_VERSION < (4, 2, 0)

This is one thing that is hard to control, the version of libwebsockets. We cannot have 4.2.0 as a minimum, to leave the interval of brokenness behind completely. Or, can we, actually?

Finally, I don't know how to select Python version in a RPM .spec, but I hopefully can figure that out.

So, just three unresolved questions, otherwise it is all clear, so far.

@jiridanek
Copy link
Contributor Author

Python 3.6 is out-of-life in upstream, https://pythonspeed.com/articles/stop-using-python-3.6/

@ganeshmurthy ganeshmurthy added this to the 3.0.0 milestone Apr 12, 2022
@jiridanek
Copy link
Contributor Author

jiridanek commented Apr 14, 2022

For RHEL 8, the situation is like this. Python 3.6 will be supported for the entire life of RHEL 8, whereas Python 3.9, etc. will have shorter support, (May 2024 for Python 3.9, to be precise), May 2024.

See https://access.redhat.com/support/policy/updates/rhel8-app-streams-life-cycle for the support info.

The problem with using Python 3.6 is with test dependencies. Many test dependencies, be it h2, quart, etc. follow upstream Python lifecycle. Sticking with Python 3.6 means we'll have to use obsolete versions of these packages in tests. If some incompatibility or other issue appears in this test setup, nobody upstream will be bothered to resolve it for us.

Selecting Python in rpm.spec discussed on https://stackoverflow.com/questions/71782123/how-do-i-install-python39-rpm-macros-for-fedora-35

jiridanek added a commit to jiridanek/skupper-router that referenced this issue Apr 30, 2022
jiridanek added a commit to jiridanek/skupper-router that referenced this issue Apr 30, 2022
jiridanek added a commit to jiridanek/skupper-router that referenced this issue Apr 30, 2022
jiridanek added a commit to jiridanek/skupper-router that referenced this issue Apr 30, 2022
jiridanek added a commit to jiridanek/skupper-router that referenced this issue Apr 30, 2022
jiridanek added a commit to jiridanek/skupper-router that referenced this issue Apr 30, 2022
jiridanek added a commit to jiridanek/skupper-router that referenced this issue Apr 30, 2022
jiridanek added a commit to jiridanek/skupper-router that referenced this issue May 2, 2022
@jiridanek jiridanek linked a pull request Jun 2, 2022 that will close this issue
jiridanek added a commit to jiridanek/skupper-router that referenced this issue Dec 6, 2023
jiridanek added a commit to jiridanek/skupper-router that referenced this issue Dec 6, 2023
jiridanek added a commit to jiridanek/skupper-router that referenced this issue Dec 6, 2023
jiridanek added a commit to jiridanek/skupper-router that referenced this issue Dec 6, 2023
jiridanek added a commit to jiridanek/skupper-router that referenced this issue Dec 6, 2023
jiridanek added a commit to jiridanek/skupper-router that referenced this issue Dec 6, 2023
jiridanek added a commit to jiridanek/skupper-router that referenced this issue Dec 6, 2023
jiridanek added a commit to jiridanek/skupper-router that referenced this issue Dec 6, 2023
@ganeshmurthy ganeshmurthy removed this from the 3.0.0 milestone Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants