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

shell.py: Fix warnings caused by overwriting cmd with str, and str too. #5697

Conversation

bernhardkaindl
Copy link
Collaborator

@bernhardkaindl bernhardkaindl commented Jun 17, 2024

@ashwin9390 @stephenchengCloud

A function in scripts/examples/python/shell.py overwrites the module cmd with a str, which results in these (not immediately obvious) warnings:

pytype: attribute-error: scripts/examples/python/shell.py#L36                                                                                                  
File "scripts/examples/python/shell.py", line 36, in __init__: No attribute 'Cmd' on str [attribute-error]
  In Union[module, str]
File "scripts/examples/python/shell.py", line 41, in preloop: No attribute 'Cmd' on str [attribute-error]
  In Union[module, str]

Let's fix it by not overwriting the module cmd, by naming the variable command instead of cmd:

cmd and command in the file after the change:

# grep -e 'cmd\>' -e command scripts/examples/python/shell.py
import cmd
class Shell(cmd.Cmd):
        cmd.Cmd.__init__(self)
        cmd.Cmd.preloop(self)
    # We want to support directly executing the cmd line,
        command = sys.argv[cmdAt]
            print(session.xenapi_request(command, tuple(params)), file=sys.stdout)

This makes it clear that with this change, from cmd to command for the last two lines, the conflict is resolved.

@bernhardkaindl bernhardkaindl force-pushed the fix-warnings-overwriting-cmd-with-str branch from f604b1b to 49397c3 Compare June 17, 2024 16:36
@psafont
Copy link
Member

psafont commented Jun 17, 2024

Shouldn't the script be moved into python3/?

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>
Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>
@bernhardkaindl bernhardkaindl force-pushed the fix-warnings-overwriting-cmd-with-str branch from 49397c3 to c2b2cf9 Compare June 17, 2024 17:30
@bernhardkaindl bernhardkaindl changed the title shell.py: Fix warning caused by overwriting cmd with str shell.py: Fix warnings caused by overwriting cmd with str, and also str itself. Jun 17, 2024
@bernhardkaindl bernhardkaindl changed the title shell.py: Fix warnings caused by overwriting cmd with str, and also str itself. shell.py: Fix warnings caused by overwriting cmd with str, and str too. Jun 17, 2024
@bernhardkaindl
Copy link
Collaborator Author

Pau asked:

Shouldn't the script be moved into python3/?

Of course, @ashwin9390 created CP-49926 for moving it into python3/ (and ~30 sibling tickets).
I did not want to interfere with the order in which he wants to move all of them.

I also added now:

  • whitespace clean-up and
  • fix the pylint warning fix for overriding the built-in type str with a variable.

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>
@psafont psafont merged commit ac91046 into xapi-project:feature/py3 Jun 25, 2024
14 checks passed
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