Package: Honor specified versions #495

Spredzy · 2020-11-17T12:59:21Z

fixes: #7841

pulpbot · 2020-11-17T13:01:37Z

Attached issue: https://pulp.plan.io/issues/7841

mikedep333

Thank you, but update the package mode docs also:
https://github.com/pulp/pulp_installer/tree/master/roles/pulp_common#role-variables-if-installing-from-rpms

Spredzy · 2020-11-30T10:33:13Z

@mikedep333 Updated. Please let me know if that matches what you expected

Spredzy · 2020-11-30T13:19:53Z

@mikedep333 @mdellweg @fao89 The failures in the CI are not code failures, but the fact that master point to pulp_version== 3.8.1 and that molecules points to https://yum.theforeman.org/pulpcore/3.7 (because 3.8 doesn't exist) and hence the error message No package python3-pulpcore = 3.8.1 available.

How would you like this to be handled ?

mikedep333 · 2020-11-30T17:13:24Z

@mikedep333 @mdellweg @fao89 The failures in the CI are not code failures, but the fact that master point to pulp_version== 3.8.1 and that molecules points to https://yum.theforeman.org/pulpcore/3.7 (because 3.8 doesn't exist) and hence the error message No package python3-pulpcore = 3.8.1 available.

How would you like this to be handled ?

Hmm, so pulp_version is a private variable actually. We do not advertise it to users, because for the pip workflow, we do not want users to override it except in rare circumstances.

I suggest we separate it into 2 variables: __pulp_pip_version and pulp_version.

pulp_version will be documented, but strongly advised against using. We'd probably list it at the bottom of the role README in a section like "variables for advanced usage only`.

pulp_version will default to undefined. __pulp_pip_version (default filter probably) will be set to it it if it is defined. __pulp_pip_version will be used for pip installs. We will maintain the value of __pulp_pip_version as we release pulp_installer releases, like we maintain pulp_version now.

For RPM installs, if pulp_version is undefined, you'll just install the latest version of pulpcore.

Spredzy · 2020-12-02T13:06:31Z

@mikedep333 @fao89 re-ready for review, with passing tests this time :)

roles/pulp_common/README.md

roles/pulp_common/defaults/main.yml

mdellweg

Looks like this needs a rebase.

roles/pulp_common/README.md

mdellweg · 2020-12-08T10:04:44Z

roles/pulp_common/defaults/main.yml

-#  pulp_version.
-#  Ignored if pulp_source_dir is set
-pulp_version: "3.8.1"
+__pulp_version: '{{ pulp_version | default("3.8.1") }}'


I believe, this should move to vars.
Also if i read it correctly, it is only used for pip installs. It should maybe be called __pulp_common_pulpcore_pip_version?

Agree on moving it to the vars/main.yml file.

For the rename, I tend to lean toward keeping __pulp_version. It is generic and accurate enough that this value can be leveraged for other purposed that is not pip related specifically.

It is only used for pip installations. And for anything else i guess one should use pulp_version.
Ideally we didn't even default_pin the python version in the installer and just install latest from pypi or the specified version.

@mikedep333 @fao89 @dkliban what are your thoughts on that one ?

@Spredzy : @mdellweg and I discussed this last meeting. There were multiple problems that pinning the version solved. This one in particular seemed to be the most tricky to solve.

Handling the case where a user may run an outdated installer to install the current version of Pulp, and thus either the install failing or pulp failing at runtime because of it.

https://pulp.plan.io/issues/5890

I am open to alternate approaches, but we should review them based on the discussions in that issue and the feedback in the initial design PR (that was centered around branches).

mdellweg · 2020-12-08T10:13:27Z

roles/pulp_common/README.md

+Role Variables for advanced usage
+---------------------------------
+
+* `pulp_version`: Specify a specific version of pulpcore one would like to install or upgrade to.


Maybe we should call it pulpcore_version.
We should be able to rename it today, since it was never advertised before.

This would be a backward incompatible change. pulp_version is already present before this PR and hence user might be leveraging it. While I agree it would make more sense, (maybe going to a deprecation period) - I don't think this should happen as part of this PR.

Changing things that are not part of an official interface is the privilege of upstream.
This variable was even described as "intentionally not advertised".

@mikedep333 @fao89 @dkliban what are your thoughts on that one ?

Yeah, I am in favor of renaming it to pulpcore_version.

fixes: #7841

Spredzy changed the title ~~Package: Allow one to specify version to install~~ Package: Honor specified versions Nov 17, 2020

Spredzy requested review from fao89, mdellweg and mikedep333 November 17, 2020 12:59

mikedep333 requested changes Nov 19, 2020

View reviewed changes

Spredzy force-pushed the specify_version_package_install branch 2 times, most recently from 7b052c6 to 3b442bd Compare November 30, 2020 10:30

Spredzy force-pushed the specify_version_package_install branch 5 times, most recently from 7696a8c to a7c406f Compare December 2, 2020 11:27

Spredzy requested a review from mikedep333 December 2, 2020 13:06

mikedep333 requested changes Dec 2, 2020

View reviewed changes

roles/pulp_common/README.md Outdated Show resolved Hide resolved

roles/pulp_common/defaults/main.yml Outdated Show resolved Hide resolved

Spredzy force-pushed the specify_version_package_install branch from a7c406f to 9c0062a Compare December 3, 2020 09:30

Spredzy requested a review from mikedep333 December 3, 2020 09:31

mdellweg reviewed Dec 8, 2020

View reviewed changes

Spredzy force-pushed the specify_version_package_install branch from 9c0062a to 3f2c1f8 Compare December 9, 2020 09:12

Spredzy requested a review from mdellweg December 9, 2020 09:15

Spredzy force-pushed the specify_version_package_install branch from 3f2c1f8 to 4298426 Compare December 9, 2020 09:17

mikedep333 approved these changes Dec 14, 2020

View reviewed changes

Spredzy force-pushed the specify_version_package_install branch from 4298426 to 5f32072 Compare December 14, 2020 19:38

mikedep333 approved these changes Dec 14, 2020

View reviewed changes

Package: Allow one to specify version to install

6804359

fixes: #7841

Spredzy force-pushed the specify_version_package_install branch from 5f32072 to 6804359 Compare December 15, 2020 16:36

Spredzy merged commit 46309a8 into pulp:master Dec 16, 2020

Package: Honor specified versions #495

Package: Honor specified versions #495

Spredzy commented Nov 17, 2020

pulpbot commented Nov 17, 2020

mikedep333 left a comment

Spredzy commented Nov 30, 2020

Spredzy commented Nov 30, 2020

mikedep333 commented Nov 30, 2020

Spredzy commented Dec 2, 2020

mdellweg left a comment

mdellweg Dec 8, 2020

Spredzy Dec 9, 2020

mdellweg Dec 9, 2020

Spredzy Dec 10, 2020

mikedep333 Dec 14, 2020

mdellweg Dec 8, 2020

Spredzy Dec 9, 2020

mdellweg Dec 9, 2020

Spredzy Dec 10, 2020

mikedep333 Dec 14, 2020

Package: Honor specified versions #495

Package: Honor specified versions #495

Conversation

Spredzy commented Nov 17, 2020

pulpbot commented Nov 17, 2020

mikedep333 left a comment

Choose a reason for hiding this comment

Spredzy commented Nov 30, 2020

Spredzy commented Nov 30, 2020

mikedep333 commented Nov 30, 2020

Spredzy commented Dec 2, 2020

mdellweg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment