Skip to content

Conversation

@dichn
Copy link

@dichn dichn commented Nov 3, 2025

No description provided.

@dichn dichn self-assigned this Nov 3, 2025
@dichn dichn marked this pull request as draft November 3, 2025 08:25
@dichn
Copy link
Author

dichn commented Nov 11, 2025

re-pushed for adding the unit test for covering CommandOptionParser.

@dichn dichn marked this pull request as ready for review November 11, 2025 13:36
@dichn
Copy link
Author

dichn commented Nov 13, 2025

@rbikar ready for code review.

@rbikar
Copy link
Member

rbikar commented Nov 25, 2025

@dichn

it looks good to me, since this is backwards incompatible change, in order to deploy new version of kobo, we would need to update rcm-pub code before that. Also other applications that build on kobo command interface.

Also I'd like to ask @kdudka to have a look. This may be potentially breaking change for some other applications.

@kdudka
Copy link
Contributor

kdudka commented Nov 25, 2025

@rbikar Good point. I have quickly looked into OpenScanHub code and we use parser.add_option(...) on many places: https://github.com/openscanhub/openscanhub/tree/main/osh/client/commands

@siteshwar @sfowl What do you think? Should we migrate OSH and coordinate it with this change in kobo?

@siteshwar
Copy link
Contributor

@siteshwar @Sfow

Is this change going to break osh-client? If not, I would not care too much about it.

@kdudka
Copy link
Contributor

kdudka commented Nov 26, 2025

Is this change going to break osh-client?

Yes, osh-client will need to be migrated, too.

OshCommand is inherited from kobo.cli.Command and it accesses self.parser, which will be an object of a different type when these changes are applied.

@sfowl
Copy link
Contributor

sfowl commented Nov 27, 2025

if this a breaking change, perhaps we should set a max version of kobo-client to buy us some time? e.g.

@ -43,6 +43,7 @@ It consists of central hub, workers and cli client.
 Summary: OpenScanHub CLI client
 Requires: koji
 Requires: python3-kobo-client >= 0.15.1-100
+Requires: python3-kobo-client <= 0.38.0
 Requires: %{name}-common = %{version}-%{release}

@kdudka
Copy link
Contributor

kdudka commented Nov 27, 2025

@sfowl This would ensure that osh-client would not traceback at run time because of this. However, Fedora and EPEL do not maintain multiple versions of a single package at a time. So once a newer version of kobo lands into stable Fedora and EPEL repos, users will not be able to install osh-client any more. For users who have osh-client already installed, dnf will refuse to upgrade kobo on their system (assuming they updated osh-client before new kobo is available).

@rbikar
Copy link
Member

rbikar commented Nov 27, 2025

@kdudka
I think that we don't have hurry with this change, but what we can do - is to keep backwards compatibility so we don't break other software that inherits from kobo.cli.Command and let maintainers to move to newer argparse variant at their own pace.

@kdudka
Copy link
Contributor

kdudka commented Nov 27, 2025

As I understand it, this would mean to maintain both the implementations in kobo in parallel during the transition period. Definitely easier for us...

@dichn
Copy link
Author

dichn commented Dec 10, 2025

@rbikar
Switching to argparse changed the structure of the parsed options, causing rcm-pub to hit an AttributeError where optparse would have exited earlier with SystemExit.

~/src/rcm-pub$ pytest
FAILED tests/integration/client/test_cmd_clear_repo.py::TestCmdClearRepo::test_command_options - AssertionError: Expected SystemExit but got <class 'AttributeError'>
FAILED tests/integration/client/test_cmd_clear_repo.py::TestCmdClearRepo::test_command_options_quay - AssertionError: Expected SystemExit but got <class 'AttributeError'>
FAILED tests/integration/client/test_cmd_clear_repo.py::TestCmdClearRepo::test_command_successful - AssertionError: Expected SystemExit but got <class 'AttributeError'>
FAILED tests/integration/client/test_cmd_clear_repo.py::TestCmdClearRepo::test_command_successful_quay - AssertionError: Expected SystemExit but got <class 'AttributeError'>
...
========= 156 failed, 497 passed

I will file a JIRA for this in rcm-pub's unit tests: RHELDST-35465

@dichn dichn marked this pull request as draft December 10, 2025 03:21
@dichn
Copy link
Author

dichn commented Dec 10, 2025

Convert it to a draft to prevent accidental merging.

@rbikar
Copy link
Member

rbikar commented Dec 12, 2025

@dichn
Can you check how difficult it would it support both optparse and argparse implementations at one time so we don't push other users to switch to argparse?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants