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.

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

[WIP] DO NOT REVIEW YET - Refactor cli and toolopts #1271

Closed
wants to merge 2 commits into from
Closed

[WIP] DO NOT REVIEW YET - Refactor cli and toolopts #1271

wants to merge 2 commits into from

Conversation

r0x0d
Copy link
Member

@r0x0d r0x0d commented Jun 19, 2024

Jira Issues:

Checklist

  • PR has been tested manually in a VM (either author or reviewer)
  • Jira issue has been made public if possible
  • [RHELC-] or [HMS-] is part of the PR title
  • GitHub label has been added to help with Release notes
  • PR title explains the change from the user's point of view
  • Code and tests are documented properly
  • The commits are squashed to as few commits as possible (without losing data)
  • When merged: Jira issue has been updated to Release Pending if relevant

@r0x0d r0x0d self-assigned this Jun 19, 2024
@r0x0d r0x0d force-pushed the refactor-cli-and-toolopts branch from 46dd982 to 4652a3b Compare June 19, 2024 17:01
github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 19, 2024
View reviewed changes
convert2rhel/unit_tests/cli_test.py

try:
cli.CLI()
except SystemExit:

Check notice

Code scanning / CodeQL

Empty except Note

'except' clause does nothing but pass and there is no explanatory comment.
convert2rhel/unit_tests/cli_test.py
),
)
def test_should_subscribe(username, password, organization, activation_key, no_rhsm, expected):
t_opts = toolopts.ToolOpts()

Check failure

Code scanning / CodeQL

Wrong number of arguments in a class instantiation Error

Call to
ToolOpts.__init__
Loading
with too few arguments; should be no fewer than 1.
convert2rhel/unit_tests/cli_test.py
)
def test_cli_args_config_file_cornercase(activation_key, organization, argv, monkeypatch):
monkeypatch.setattr(sys, "argv", mock_cli_arguments(argv))
t_opts = toolopts.ToolOpts()

Check failure

Code scanning / CodeQL

Wrong number of arguments in a class instantiation Error

Call to
ToolOpts.__init__
Loading
with too few arguments; should be no fewer than 1.
convert2rhel/cli.py Dismissed Show dismissed Hide dismissed
convert2rhel/unit_tests/cli_test.py
import pytest
import six

from convert2rhel import cli, toolopts, utils

Check notice

Code scanning / CodeQL

Unused import Note

Import of 'utils' is not used.
convert2rhel/cli.py
Comment on lines +373 to +381
# if (parsed_opts.password or config_opts.password) and not (parsed_opts.username or config_opts.username):
# loggerinst.warning(
# "You have passed the RHSM password without an associated username. Please provide a username together"
# " with the password."

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
convert2rhel/cli.py
Comment on lines +379 to +387
# if (parsed_opts.username or config_opts.username) and not (parsed_opts.password or config_opts.password):
# loggerinst.warning(
# "You have passed the RHSM username without an associated password. Please provide a password together"
# " with the username."

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
convert2rhel/unit_tests/cli_test.py
file.write(content)
os.chmod(path, 0o600)

opts = cli.options_from_config_files(path)

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable opts is not used.
convert2rhel/cli.py
return args


def _subcommand_used(args):

Check notice

Code scanning / CodeQL

Explicit returns mixed with implicit (fall through) returns Note

Mixing implicit and explicit returns may indicate an error as implicit returns always return None.
convert2rhel/utils/subscription.py
loggerinst = logging.getLogger(__name__)


def setup_rhsm_parts(opts):

Check notice

Code scanning / CodeQL

Explicit returns mixed with implicit (fall through) returns Note

Mixing implicit and explicit returns may indicate an error as implicit returns always return None.
r0x0d
r0x0d commented Jun 19, 2024
View reviewed changes
config/convert2rhel.ini
@@ -11,3 +11,12 @@
# password = <insert_password>
# activation_key = <insert_activation_key>
# org = <insert_org>

[settings]
Copy link
Member Author

Choose a reason for hiding this comment

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

Will be updated with #1272

@r0x0d r0x0d force-pushed the refactor-cli-and-toolopts branch from 4652a3b to 997993b Compare June 20, 2024 12:19
github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 20, 2024
View reviewed changes
convert2rhel/cli.py

import argparse
import logging
import os

Check notice

Code scanning / CodeQL

Unused import Note

Import of 'os' is not used.
@r0x0d
Copy link
Member Author

r0x0d commented Sep 3, 2024

Dropping this in favor of #1272.

There is no point in having this enhancement in a separate PR since we are past the release now. We could just go with #1272 and make it feature complete.

@r0x0d r0x0d closed this Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@r0x0d r0x0d

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@r0x0d