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

verdi node graph generate error for missing PK is not correctly handled and help page is wrong #6397

Closed
agoscinski opened this issue May 16, 2024 · 1 comment · Fixed by #6427
Assignees
Labels

Comments

@agoscinski
Copy link
Contributor

Describe the bug / Steps to reproduce / Expected behavior

  • Error for missing is PK not properly handled
$ verdi node graph generate
Traceback (most recent call last):
  File "/home/alexgo/micromamba/envs/aiida-dev/bin/verdi", line 8, in <module>
    sys.exit(verdi())
             ^^^^^^^
  File "/home/alexgo/micromamba/envs/aiida-dev/lib/python3.11/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alexgo/micromamba/envs/aiida-dev/lib/python3.11/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/home/alexgo/micromamba/envs/aiida-dev/lib/python3.11/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alexgo/micromamba/envs/aiida-dev/lib/python3.11/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alexgo/micromamba/envs/aiida-dev/lib/python3.11/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alexgo/micromamba/envs/aiida-dev/lib/python3.11/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alexgo/micromamba/envs/aiida-dev/lib/python3.11/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alexgo/code/aiida-core/src/aiida/cmdline/utils/decorators.py", line 102, in wrapper
    return wrapped(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alexgo/code/aiida-core/src/aiida/cmdline/commands/cmd_node.py", line 535, in graph_generate
    pks = '.'.join(str(n.pk) for n in root_nodes)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alexgo/code/aiida-core/src/aiida/cmdline/commands/cmd_node.py", line 535, in <genexpr>
    pks = '.'.join(str(n.pk) for n in root_nodes)
                       ^^^^
AttributeError: 'NoneType' object has no attribute 'pk'

Compare to

$ verdi computer show
Usage: verdi computer show [OPTIONS] COMPUTER
Try 'verdi computer show --help' for help.

Error: Missing argument 'COMPUTER'.

The error seems not to be properly handled in the former case.

  • The help page of the command is wrong
$ verdi node graph generate --help
Usage: verdi node graph generate [OPTIONS] [--] [ROOT_NODE] OUTPUT_FILE

  Generate a graph from one or multiple root nodes.
[...]

Again comparing it with other commands, I interpret [ROOT_NODE] as optional, but it is not, it is the OUTPUT_FILE which is optional. Also verdi node graph generate -N 1 output.pdf does not work but one has to do verdi node graph generate -N 1 -O output.pdf

Your environment

  • Operating system: Ubuntu 22.04 LTS
  • Python version: 3.11
$ verdi status
✔ version:     AiiDA v2.5.1
✔ config:      /home/alexgo/code/aiida-core/.aiida
✔ profile:     alexgo
✔ storage:     SqliteDosStorage[/home/alexgo/code/aiida-core/.aiida/repository/sqlite_dos_f275ff0f10174e8e84c13e82dd5ba452]: open,
✔ rabbitmq:    Connected to RabbitMQ v3.8.14 as amqp://guest:[email protected]:5672?heartbeat=600
@sphuber
Copy link
Contributor

sphuber commented May 20, 2024

This is a regression introduced by #6338 which made it possible to specify multiple root nodes using the -N option. Specifying the node positionally was deprecated and had to be made optional, so the help string is correct. It is generated automatically by click by the way.

The problem here is the handling of the case where no root node is specified neither positionally nor with -N.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants