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.

Sign up for GitHub

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

Jump to bottom

fix(clusterstatus): Update cli output #7

Merged
merged 1 commit into from
Nov 6, 2024
Merged

fix(clusterstatus): Update cli output #7

merged 1 commit into from
Nov 6, 2024

Conversation

lperdereau
Copy link
Collaborator

No description provided.

@lperdereau
Copy link
Collaborator Author

lperdereau commented Oct 29, 2024 •
edited
Loading

Current rendered output

INFO:root:Proxmox cluster: devel

  Status: healthy
  VMs: 8
  Templates: 6
  Metrics:
    CPU: 0.01/64(0.0%), allocated: 6
    Memory: 9.67 GiB/314.67 GiB(3.1%), allocated: 6.00 GiB
    Disk: 39.88 GiB/2.66 TiB(1.5%)
  Nodes:
    Offline: 0
    Online: 2
    Unknown: 0

@lperdereau lperdereau marked this pull request as ready for review November 4, 2024 09:40
plaffitt
plaffitt reviewed Nov 4, 2024
View reviewed changes
src/pvecontrol/__init__.py Outdated

from pvecontrol.node import PVENode
from pvecontrol.node import NodeStatus
from pvecontrol.vm import PVEVm
from pvecontrol.vm import VmStatus
from pvecontrol.task import PVETask
from pvecontrol.utils import _print_tableoutput, _filter_keys
Copy link
Collaborator

Choose a reason for hiding this comment

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

In regard to PEP 8, underscore prefixed names suggest an internal/private use. You should rename those I think.

_single_leading_underscore: weak “internal use” indicator. E.g. from M import * does not import objects whose names start with an underscore.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep _ means "local" which does not exists in python :/

@lcaflc lcaflc changed the base branch from main to dev November 4, 2024 16:41
@lperdereau lperdereau force-pushed the summary branch 2 times, most recently from 67dabd7 to eb5e774 Compare November 5, 2024 06:10
@lperdereau lperdereau changed the title feat: Add summary command fix(clusterstatus): Update cli output Nov 5, 2024
@lperdereau
Copy link
Collaborator Author

@paullaffitte @lcaflc do we need json / yaml output ?

lcaflc
lcaflc approved these changes Nov 6, 2024
View reviewed changes
Copy link
Contributor

@lcaflc lcaflc left a comment

Choose a reason for hiding this comment

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

All seems OK. approved for dev validation

@lcaflc lcaflc merged commit 23d3aa7 into dev Nov 6, 2024
2 checks passed
@lcaflc lcaflc deleted the summary branch November 6, 2024 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@plaffitt plaffitt plaffitt left review comments

@lcaflc lcaflc lcaflc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lperdereau @lcaflc @plaffitt