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

mgr: add urllib3==1.26.15 to mgr/requirements.txt #51298

Merged
merged 1 commit into from May 1, 2023

Conversation

ljflores
Copy link
Contributor

@ljflores ljflores commented May 1, 2023

We do not depend on any particular version of urllib3, but as a workaround to the incompatibility of urllib3 constraints between kubernetes and requests, we need to pin it temporarily to the version both are happy with.

When requests is compatible with a higher urllib3 version, we can unpin it again.

See https://tracker.ceph.com/issues/48210 for the history of this happening before.

Fixes: https://tracker.ceph.com/issues/59591
Signed-off-by: Laura Flores lflores@redhat.com

Contribution Guidelines

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

We do not depend on any particular version of
urllib3, but as a workaround to the incompatibility
of urllib3 constraints between kubernetes and
requests, we need to pin it temporarily to
the version both are happy with.

Fixes: https://tracker.ceph.com/issues/59591
Signed-off-by: Laura Flores <lflores@redhat.com>
@adk3798
Copy link
Contributor

adk3798 commented May 1, 2023

2289ad2 seemed to imply the main reason for setting the constraint on the version of requests was because it needed to be at least 2.26. I wonder if it's possible to relax that requirement to something like >=2.26 and if that would make a difference. The patch in this PR has gotten the setup-venv-for-mgr passing though so maybe it's best to just do this in order to unblock make check. Just thinking with this we'll have to remember to one day remove this pin the same as what happened with https://tracker.ceph.com/issues/48210

@ljflores ljflores requested a review from dmick May 1, 2023 17:34
Copy link
Contributor

@adk3798 adk3798 left a comment

Choose a reason for hiding this comment

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

Gets make check passing, so as a temporary solution LGTM

@ljflores
Copy link
Contributor Author

ljflores commented May 1, 2023

Going to merge this as a workaround for now. I'm also adding a note to the tracker that if we experience any issues in the future with urllib3, we should consider unpinning it.

@ljflores ljflores merged commit 3d33b9a into ceph:main May 1, 2023
18 checks passed
@ljflores ljflores deleted the wip-urllib3-version branch May 1, 2023 19:30
@dmick
Copy link
Member

dmick commented May 1, 2023

agreed that I don't know why requests was exactly pinned to 2.26, but, pinning is so evil anyway

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants