Skip to content

(PUP-1537) add hold attribute #7773

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

(PUP-1537) add hold attribute #7773

wants to merge 1 commit into from

Conversation

@ciprianbadescu
Copy link
Contributor

@ciprianbadescu ciprianbadescu commented Oct 8, 2019 •
edited
Loading

This commit adds hold property for package providers on
Debian(dpkg/apt/fink/aptitude) and Solaris(pgk) as
alternative of using "held" value for ensure.

Using "held" value for ensure works as before, but it shows
deprecation warning and will be removed in further release.

Allowed values for hold are "yes"/"true"/"no"/"false",
default to "false". Hold can be specified with or without ensure,
if ensure is missing will default to "present".

Hold cannot be specified together with "purged", "absent" or "held"
values for ensure.

@ciprianbadescu ciprianbadescu requested review from a team October 8, 2019 11:36
@ciprianbadescu ciprianbadescu force-pushed the PUP-1537/dpkg-hold-feature branch from b4249b2 to bbebede Compare October 8, 2019 12:38
@puppetcla
Copy link

puppetcla commented Oct 8, 2019

CLA signed by all contributors.

@ciprianbadescu ciprianbadescu force-pushed the PUP-1537/dpkg-hold-feature branch from bbebede to 005ff9a Compare October 11, 2019 12:10
@ciprianbadescu
Copy link
Contributor Author

ciprianbadescu commented Oct 11, 2019

jenkins please test this on debian8-64a,debian8-32a,debian9-64a,debian9-32a,solaris10-64a,solaris114-64a,ubuntu1404-64a,ubuntu1404-32a,ubuntu1604-64a,ubuntu1604-32a,ubuntu1804-64a

@ciprianbadescu
Copy link
Contributor Author

ciprianbadescu commented Oct 11, 2019

jenkins please test this on
debian8-64a,debian8-32a,debian9-64a,debian9-32a,ubuntu1604-64a,ubuntu1604-32a,ubuntu1804-64a,solaris10-64a,solaris114-64a

@ciprianbadescu ciprianbadescu force-pushed the PUP-1537/dpkg-hold-feature branch from 005ff9a to 119f5ae Compare October 14, 2019 13:17
@ciprianbadescu
Copy link
Contributor Author

ciprianbadescu commented Oct 16, 2019

It looks like originally the remove step is not really handled, I will add fix this this before merging

joshcooper
joshcooper reviewed Oct 23, 2019
View reviewed changes
joshcooper
joshcooper reviewed Oct 23, 2019
View reviewed changes
joshcooper
joshcooper reviewed Oct 23, 2019
View reviewed changes
joshcooper
joshcooper reviewed Oct 23, 2019
View reviewed changes
@ciprianbadescu ciprianbadescu force-pushed the PUP-1537/dpkg-hold-feature branch 9 times, most recently from 32a636e to bc3f287 Compare October 28, 2019 15:01
@ciprianbadescu
Copy link
Contributor Author

ciprianbadescu commented Oct 28, 2019

jenkins please test this on
debian8-64a,debian8-32a,debian9-64a,debian9-32a,solaris10-64a,solaris114-64a,ubuntu140464a,ubuntu1404-32a,ubuntu1604-64a,ubuntu1604-32a,ubuntu1804-64a

joshcooper
joshcooper reviewed Oct 30, 2019
View reviewed changes
joshcooper
joshcooper requested changes Oct 30, 2019
View reviewed changes
Copy link
Contributor

@joshcooper joshcooper left a comment

Choose a reason for hiding this comment

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

We shouldn't modify the resource harness for this one use case. Need to solve the boolean parameter and property bugs in a way that doesn't break providers.

@ciprianbadescu ciprianbadescu force-pushed the PUP-1537/dpkg-hold-feature branch 3 times, most recently from 311a20f to 6a12662 Compare October 31, 2019 13:13
@ciprianbadescu
Copy link
Contributor Author

ciprianbadescu commented Oct 31, 2019

jenkins please test this on
debian8-64a,debian8-32a,debian9-64a,debian9-32a,ubuntu1604-64a,ubuntu1604-32a,ubuntu1804-64a,solaris10-64a,solaris114-64a

joshcooper
joshcooper reviewed Oct 31, 2019
View reviewed changes
joshcooper
joshcooper reviewed Oct 31, 2019
View reviewed changes
@ciprianbadescu ciprianbadescu force-pushed the PUP-1537/dpkg-hold-feature branch 2 times, most recently from 8eca10b to ecfcb8f Compare November 4, 2019 08:40
(PUP-1537) add hold property for package
073a6dd 
This commit adds `hold` property for package providers on
Debian(dpkg/apt/fink/aptitude) and Solaris(pgk) as
alternative of using "held" value for `ensure`.

Using "held" value for `ensure` works as before, but it shows
deprecation warning and will be removed in further release.

Allowed values for `hold` are "yes"/"true"/"no"/"false",
default to "false". Hold can be specified with or without `ensure`,
if `ensure` is missing will default to "present".

Hold cannot be specified together with "purged", "absent" or "held"
values for `ensure`.
@ciprianbadescu ciprianbadescu force-pushed the PUP-1537/dpkg-hold-feature branch from ecfcb8f to 073a6dd Compare November 4, 2019 10:54
@ciprianbadescu ciprianbadescu mentioned this pull request Nov 4, 2019
Merged
@ciprianbadescu
Copy link
Contributor Author

ciprianbadescu commented Nov 4, 2019

@joshcooper, does it makes sense to extend/rename above hold property and functionality with a mark property accepting values manual/hold/none values?

joshcooper
joshcooper reviewed Nov 4, 2019
View reviewed changes
lib/puppet/type/package.rb

def should_to_s(value)
(value == :true).to_s
end
Copy link
Contributor

@joshcooper joshcooper Nov 4, 2019

Choose a reason for hiding this comment

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

Does is_to_s need to be overridden?

Copy link
Contributor Author

@ciprianbadescu ciprianbadescu Nov 6, 2019

Choose a reason for hiding this comment

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

without it will print something like:
Changing hold from "false" to true

@joshcooper
Copy link
Contributor

joshcooper commented Nov 4, 2019

property accepting values manual/hold/none values?

I'm not familiar enough with what those mean to say if they should be a single parameter or different. Suggest asking in #puppet-dev slack.

@gimmyxd gimmyxd mentioned this pull request Nov 5, 2019
Closed
@ciprianbadescu
Copy link
Contributor Author

ciprianbadescu commented Nov 6, 2019

Will prepare a second PR replacing hold=[true|false] with mark=[hold|none] to allow extension with further values of mark on apt provider.

@ciprianbadescu
Copy link
Contributor Author

ciprianbadescu commented Nov 8, 2019

work to be continued on: #7815

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joshcooper joshcooper joshcooper requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ciprianbadescu @puppetcla @joshcooper