Skip to content

install-deps.sh: remove line that updates pip#52558

Closed
ljflores wants to merge 1 commit intoceph:mainfrom
ljflores:wip-fix-make-check
Closed

install-deps.sh: remove line that updates pip#52558
ljflores wants to merge 1 commit intoceph:mainfrom
ljflores:wip-fix-make-check

Conversation

@ljflores
Copy link
Member

@ljflores ljflores commented Jul 19, 2023

This came from the new release of pip on July 15th: https://pypi.org/project/pip/23.2/

Keeping pip at v22.0.2 is what is helping the tests pass for now.

Fixes: https://tracker.ceph.com/issues/62082

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

@ljflores ljflores requested a review from a team as a code owner July 19, 2023 23:31
@ljflores ljflores requested review from avanthakkar and nizamial09 and removed request for a team July 19, 2023 23:31
@ljflores ljflores force-pushed the wip-fix-make-check branch 2 times, most recently from 5b06d59 to 2fd0b0f Compare July 19, 2023 23:59
Somehow, updating pip causes packages to go missing
in various generated /wheelhouse folders.

Fixes: https://tracker.ceph.com/issues/62082
Signed-off-by: Laura <lflores@redhat.com>
@ljflores ljflores force-pushed the wip-fix-make-check branch from 2fd0b0f to adc389b Compare July 20, 2023 03:59
@ljflores ljflores changed the title pybind, python-common: pin dependencies to compatible versions install-deps.sh: remove line that updates pip Jul 20, 2023
@ljflores
Copy link
Member Author

jenkins test api

1 similar comment
@ljflores
Copy link
Member Author

jenkins test api

@nizamial09
Copy link
Member

Hey @ljflores sorry i went to sleep yesterday before updating the tracker but i have an update to this PR. #52544. Could you have a look?

@ljflores
Copy link
Member Author

jenkins test api

type python3 > /dev/null 2>&1 || continue
activate_virtualenv $top_srcdir || exit 1
python3 -m pip install --upgrade pip
# python3 -m pip install --upgrade pip
Copy link
Member

Choose a reason for hiding this comment

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

not sure whether its a good idea to stop pip from upgrading. Maybe pinning it to 22.02 rather than stopping it? Also when newer python versions are applied to project, we might need new pip to pull in the new dependencies which might be unsupported by older pip versions. But I am not a best person to speak for python. I'll let other people to decide.

We are also updating pip in the src/tools/setup-venv.sh file. So I guess we are limitting it while on the install-deps only and this causes differences between the versions while installing install-deps deps and tox deps.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe pinning it to 22.02 rather than stopping it?

Do we know what changed after that release?

Also when newer python versions are applied to project, we might need new pip to pull in the new dependencies which might be unsupported by older pip versions.

Exactly. This is why I like #52544 better.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is why I like #52544 better.

It also gets rid of "fast-deps has no effect when used with the legacy resolver" warnings.

Copy link
Member

Choose a reason for hiding this comment

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

Do we know what changed after that release?

I couldn't find why this changed suddenly. But looks like there have been so many deprecations or changes happening with the legacy versions on some of the latest pip releases

Copy link
Contributor

Choose a reason for hiding this comment

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

I also prefer to fix this problem for the newest pip, but we may also want to pin it to a specific major version (something like >=23,<24 IIRC) so that as long as pip is well behaved and don't make big behavior changes in minor versions we get the benefit of taking fixes but not getting surprised by major version changes. It just requires some discipline to periodically check that there's a new major version and then update+test.

Copy link
Contributor

@idryomov idryomov Jul 20, 2023

Choose a reason for hiding this comment

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

Here it broke with a minor version: 23.2 was released five days ago, the issue popped up immediately after that. I didn't dig deeper but it seems to be too close to be just a coincidence.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I missed that. I had thought the behavior change was going from 22.x to 23.x, thanks for the update.

@ljflores
Copy link
Member Author

Closing in favor of #52544. Thanks @nizamial09 !

@ljflores ljflores closed this Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants