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

fix: update Uname parser to fix LooseVersion comparision error #3814

Merged
merged 4 commits into from
Aug 9, 2023

Conversation

qinpingli
Copy link
Contributor

@qinpingli qinpingli commented Jun 8, 2023

Detailed error is:
TypeError: '<' not supported between instances of 'str' and 'int'

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:

The issue is: #3815

@qinpingli qinpingli force-pushed the RHINRULE-198 branch 2 times, most recently from 39939fd to e949e18 Compare June 29, 2023 01:33
@qinpingli qinpingli changed the title [WIP] fix: update Uname parser to fix LooseVersion comparision error fix: update Uname parser to fix LooseVersion comparision error Jun 29, 2023
@qinpingli
Copy link
Contributor Author

@xiangce @chenlizhong please help review, thanks!

@xiangce xiangce added the BREAK Rules The change breaks the test of some rules. label Jul 12, 2023
@qinpingli qinpingli force-pushed the RHINRULE-198 branch 4 times, most recently from 1e2b25b to 8842a7b Compare July 27, 2023 00:14
Copy link
Contributor

@xiangce xiangce left a comment

Choose a reason for hiding this comment

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

Since the pad_release method is also updated, please add tests to cover the new updates.

insights/parsers/uname.py Outdated Show resolved Hide resolved
insights/parsers/uname.py Outdated Show resolved Hide resolved
Exception: TypeError: '<' not supported between instances of 'str' and 'int'
is returned when the fixed_by or introduced_in doesn't have distribution part
in release and version and other release parts are the same as current kernel.

The detailed update is not doing comparation the distribution part in release.

Signed-off-by: Ping Qin <[email protected]>
When _lv_release in self and other both or neither have the distribution
part, compare them directly. Otherwise, removing the distribution part
first, then doing the comparation without the distribution part and
raising a warning.

Signed-off-by: Ping Qin <[email protected]>
@xiangce
Copy link
Contributor

xiangce commented Aug 9, 2023

Since the pad_release method is also updated, please add tests to cover the new updates.

@qinpingli - you may miss this comment? and btw, since a warning message was added to show for the fixed_by, better add more tests for it too.

@qinpingli
Copy link
Contributor Author

Since the pad_release method is also updated, please add tests to cover the new updates.

@qinpingli - you may miss this comment? and btw, since a warning message was added to show for the fixed_by, better add more tests for it too.

You are right! Sorry for missing it. Now, new test cases are added, please help take a look again, thanks!

@qinpingli qinpingli force-pushed the RHINRULE-198 branch 2 times, most recently from ffc8155 to 0da0096 Compare August 9, 2023 03:16
@qinpingli qinpingli force-pushed the RHINRULE-198 branch 3 times, most recently from 7aca83a to 5c9cc3b Compare August 9, 2023 03:59
@xiangce xiangce merged commit 0862442 into RedHatInsights:master Aug 9, 2023
7 checks passed
xiangce pushed a commit that referenced this pull request Aug 9, 2023
* fix: enhance Uname.fixed_by()

Exception: TypeError: '<' not supported between instances of 'str' and 'int'
is returned when the fixed_by or introduced_in doesn't have distribution part
in release and version and other release parts are the same as current kernel.

The detailed update is not doing comparation the distribution part in release.

Signed-off-by: Ping Qin <[email protected]>

* fix: Update Uname.fixed_by()

When _lv_release in self and other both or neither have the distribution
part, compare them directly. Otherwise, removing the distribution part
first, then doing the comparation without the distribution part and
raising a warning.

Signed-off-by: Ping Qin <[email protected]>

* fix: Add test for pad_release() and warnings in fixed_by()
* fix: seperate the test cases for warnings in a different method

Signed-off-by: Ping Qin <[email protected]>
(cherry picked from commit 0862442)
xiangce pushed a commit that referenced this pull request Sep 6, 2024
* fix: enhance Uname.fixed_by()

Exception: TypeError: '<' not supported between instances of 'str' and 'int'
is returned when the fixed_by or introduced_in doesn't have distribution part
in release and version and other release parts are the same as current kernel.

The detailed update is not doing comparation the distribution part in release.

Signed-off-by: Ping Qin <[email protected]>

* fix: Update Uname.fixed_by()

When _lv_release in self and other both or neither have the distribution
part, compare them directly. Otherwise, removing the distribution part
first, then doing the comparation without the distribution part and
raising a warning.

Signed-off-by: Ping Qin <[email protected]>

* fix: Add test for pad_release() and warnings in fixed_by()
* fix: seperate the test cases for warnings in a different method

Signed-off-by: Ping Qin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAK Rules The change breaks the test of some rules.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants