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

Fixed check_ceph_osd for Ceph Octopus. #83

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

HeroesLament
Copy link

Removed unnecessary parentheses for Python 3, fixed regex variable import, and added a loop to the print statements to properly format the aggregate OSD states.

The findall regex function of python3 outputs a tuple when searching for multiple statements. This is in contrast to prior versions which output an array of strings, requiring us to loop through joining the tuples prior to printing.

mwynkoop01 added 2 commits May 2, 2022 13:33
Removed unnecessary parentheses for Python 3, fixed regex variable import, and added a loop to the print statements to properly format the aggregate OSD states.

The findall regex function of python3 outputs a tuple when searching for multiple statements. This is in contrast to prior versions which output an array of strings, requiring us to loop through joining the tuples prior to printing.
@valerytschopp
Copy link
Contributor

Hello @HeroesLament,
The parentheses in the print statements are NOT optional. In python3 it is mandatory, print(...) is a function call...
Example:

Python 3.9.10 (main, Jan 15 2022, 11:48:00) 
>>> print "hello world!"
  File "<stdin>", line 1
    print "hello world!"
          ^
SyntaxError: Missing parentheses in call to 'print'. Did you mean print("hello world!")?

Copy link
Contributor

@valerytschopp valerytschopp left a comment

Choose a reason for hiding this comment

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

I can not merge you PR:

  • Wrong python3 print statements
  • Commands not executed

So please change your PR accordingly, and I will re-consider it

@@ -55,26 +54,26 @@ def main():
# validate args
ceph_exec = args.exe if args.exe else CEPH_COMMAND
if not os.path.exists(ceph_exec):
print("OSD ERROR: ceph executable '%s' doesn't exist" % ceph_exec)
Copy link
Contributor

Choose a reason for hiding this comment

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

Python3 print requires parentheses!

print("OSD ERROR: %s" % err)
return STATUS_ERROR
# exec command
print ceph_cmd
Copy link
Contributor

Choose a reason for hiding this comment

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

You just print the ceph_cmd?!? Where is it executed?

@HeroesLament
Copy link
Author

Hello @valerytschopp ,

The work our team was doing on this was apparently not on track. The server this was being worked on was running python 2.7, so much of the syntax was not correct. We are working to re-factor this with python 3.8 now.

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.

3 participants