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

Remove usage of reregistration and deprecate cli-option #3522

Merged
merged 1 commit into from
Dec 7, 2022
Merged

Remove usage of reregistration and deprecate cli-option #3522

merged 1 commit into from
Dec 7, 2022

Conversation

ahitacat
Copy link
Contributor

Signed-off-by: ahitacat [email protected]

All Pull Requests:

Check all that apply:

  • Have you followed the guidelines in our Contributing document, including the instructions about commit messages?
  • Is this PR to correct an issue?
  • Is this PR an enhancement?

Complete Description of Additions/Changes:

Working on deprecate --force-reregistration client option. This option will be deprecate in favor of the explicit running of insights-client --unregister and insights-client --register. This work is part of the improvement of the re-registration safeguard [RHIN-500]

Resolves: #rhbz2071082

@ahitacat ahitacat added the client These issues represent work to be done by the "client" team. label Sep 13, 2022
@ahitacat ahitacat requested a review from strider September 13, 2022 10:45
@ahitacat ahitacat self-assigned this Sep 13, 2022
@clarabez
Copy link

LGTM!

@@ -326,7 +326,7 @@ def _core_collect_default():
'reregister': {
'default': False,
'opt': ['--force-reregister'],
'help': 'Forcefully reregister this machine to Red Hat. Use only as directed.',
'help': argparse.SUPPRESS,
Copy link
Contributor

Choose a reason for hiding this comment

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

As we are in deprecation mode, I will not silence that option from argparse now. But I would suggest adding the deprecation message, announcing the ineffectiveness of that latter and its future removal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, I have added a new message

@ahitacat ahitacat changed the title Remove usage of reregistration and deprecate cli-option [do not merge] Remove usage of reregistration and deprecate cli-option Sep 27, 2022
@ahitacat ahitacat requested a review from strider September 27, 2022 13:16
Copy link
Contributor

@strider strider left a comment

Choose a reason for hiding this comment

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

LGTM

@ahitacat ahitacat marked this pull request as ready for review December 7, 2022 10:38
@ahitacat ahitacat changed the title [do not merge] Remove usage of reregistration and deprecate cli-option Remove usage of reregistration and deprecate cli-option Dec 7, 2022
@subpop subpop merged commit 3a8e54c into RedHatInsights:master Dec 7, 2022
subpop pushed a commit that referenced this pull request Dec 7, 2022
xiangce pushed a commit that referenced this pull request Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client These issues represent work to be done by the "client" team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants