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 "RET503", # Missing explicit return at the end of function able to return non-None value #1357

Merged
merged 3 commits into from
Apr 8, 2024

Conversation

jiridanek
Copy link
Member

No description provided.

… to return non-`None` value

Signed-off-by: Jiri Daněk <[email protected]>
@jiridanek jiridanek added the enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) label Apr 8, 2024
@jiridanek jiridanek self-assigned this Apr 8, 2024
Copy link
Contributor

github-actions bot commented Apr 8, 2024

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass %
456 0 0 456 100

jstourac
jstourac previously approved these changes Apr 8, 2024
kobihk
kobihk previously requested changes Apr 8, 2024
ods_ci/utils/scripts/ocm/ocm.py Outdated Show resolved Hide resolved
ods_ci/utils/scripts/ocm/ocm.py Outdated Show resolved Hide resolved
ods_ci/utils/scripts/ocm/ocm.py Outdated Show resolved Hide resolved
ods_ci/utils/scripts/ocm/ocm.py Outdated Show resolved Hide resolved
ods_ci/utils/scripts/rosa/rosaOps.py Outdated Show resolved Hide resolved
Signed-off-by: Jiri Daněk <[email protected]>
Signed-off-by: Jiri Daněk <[email protected]>
Copy link

sonarqubecloud bot commented Apr 8, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
1.8% Duplication on New Code

See analysis details on SonarCloud

@jiridanek jiridanek requested review from kobihk and jstourac April 8, 2024 12:39
@jiridanek jiridanek dismissed kobihk’s stale review April 8, 2024 13:40

changes made

@jiridanek jiridanek requested a review from diegolovison April 8, 2024 15:08
@dhirajsb dhirajsb requested review from dhirajsb and removed request for dhirajsb April 8, 2024 16:01
@dhirajsb
Copy link

dhirajsb commented Apr 8, 2024

I'm not sure why I'm being asked to review this? Did you mean to request a review from someone else?

@jiridanek
Copy link
Member Author

I'm not sure why I'm being asked to review this? Did you mean to request a review from someone else?

I clicked all the autosuggestions from github, should be based on who edited the files recently. It's possible i missclicked. If so, sorry about the disturbance.

@dhirajsb dhirajsb removed their request for review April 8, 2024 16:06
@jiridanek jiridanek merged commit b791898 into red-hat-data-services:master Apr 8, 2024
10 checks passed
@jstourac
Copy link
Member

jstourac commented Apr 9, 2024

@jiridanek has this been tested indeed? I can see following failure in our job 🤔

07:30:59  + python3 /home/jenkins/workspace/rhods/exploratory-test-rhods-deploy/ods-ci/ods_ci/utils/scripts/ocm/ocm.py ocm_login --token ****
07:30:59  Traceback (most recent call last):
07:30:59    File "/home/jenkins/workspace/rhods/exploratory-test-rhods-deploy/ods-ci/ods_ci/utils/scripts/ocm/ocm.py", line 28, in <module>
07:30:59      class OpenshiftClusterManager:
07:30:59    File "/home/jenkins/workspace/rhods/exploratory-test-rhods-deploy/ods-ci/ods_ci/utils/scripts/ocm/ocm.py", line 1186, in OpenshiftClusterManager
07:30:59      def change_cluster_channel_group(self) -> str | None:
07:30:59  TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'

@jiridanek
Copy link
Member Author

The initial version was semantically equivalent code. The changes I did at @kobihk 's suggestion do have potential to break things. And no, I did not test it before merge.

@jiridanek
Copy link
Member Author

jiridanek commented Apr 9, 2024

The fix is to add

from __future__ import annotations

at the top of the file. Be cause file is nor run with python3 11 but with some older python.

I'll put it in when I get to the office in 1.30 hours or so.

jstourac added a commit to jstourac/ods-ci that referenced this pull request Apr 9, 2024
…ion able to return non-`None` value (red-hat-data-services#1357)"

This reverts commit b791898.

This change caused a TypeError issue when trying to do the ocm login,
see [1]

[1] red-hat-data-services#1357 (comment)
jstourac added a commit that referenced this pull request Apr 9, 2024
…ion able to return non-`None` value (#1357)"

This reverts commit b791898.

This change caused a TypeError issue when trying to do the ocm login,
see [1]

[1] #1357 (comment)
@kobihk
Copy link
Contributor

kobihk commented Apr 9, 2024

The initial version was semantically equivalent code. The changes I did at @kobihk 's suggestion do have potential to break things. And no, I did not test it before merge.

@jiridanek maybe without the definition of return type it could work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants