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

Prs/adding pickle output to cli #64

Merged
merged 29 commits into from
Sep 23, 2024
Merged

Conversation

boekhorstb1
Copy link
Contributor

see issue: #60

@boekhorstb1 boekhorstb1 self-assigned this Jun 25, 2024
@boekhorstb1
Copy link
Contributor Author

You can use the cli like so:

rookify --show
rookify --show device

the later will parse for the device section. If a section does not exist an error is thrown.
I did not completely udnerstand what was meant with the 'migrated' flags, probably need to still add that

@NotTheEvilOne
Copy link
Contributor

In general I like the option to filter outputs. What needs to be changed is that each module should present it's understanding of the state data on it's own instead of an "one fits all" approach to just output everything as a JSON blob.

@boekhorstb1 boekhorstb1 force-pushed the prs/adding-pickle-output-to-cli branch 2 times, most recently from bbac0c4 to 821a914 Compare September 10, 2024 14:03
@boekhorstb1
Copy link
Contributor Author

Ok cli has some more features, most of all the show-progress feature:

.venv/bin/rookify --help
usage: Rookify [-h] [--dry-run] [--list-modules] [--read-pickle [<section>]] [--show-progress [<module>]]

options:
  -h, --help            show this help message and exit
  --dry-run
  --list-modules        List all modules
  --read-pickle [<section>]
                        Show the content of the pickle file. Default argument is 'all', you can also specify a section you want to look at.
  --show-progress [<module>]
                        Show progress of the modules.

I only implemented it for analyze_ceph, as I think I probably should add a status-mode in order to proceed or do you think I should pass the argument through loadmodules(....,show_progress=True)?

Running of rookify --show_progress now looks like this:

.venv/bin/rookify --show-progress
2024-09-12 14:11.09 [info     ] Pickle file set: data.pickle  
2024-09-12 14:11.09 [info     ] Pickle file set: data.pickle  
2024-09-12 14:11.09 [info     ] Show progress of all modules  
2024-09-12 14:11.09 [info     ] Execution started with machine pickle file
2024-09-12 14:11.09 [info     ] AnalyzeCephHandler Progress: Not all commands have been run yet.

And I was thinking, that it would be nice to have extra arguments to run the command for each module to check the status.'

Also added a lot of tests

@NotTheEvilOne NotTheEvilOne self-assigned this Sep 17, 2024
Copy link
Contributor

@NotTheEvilOne NotTheEvilOne left a comment

Choose a reason for hiding this comment

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

LGTM

@boekhorstb1: Please check if you want to add or change anything on your adapted changes :) Feel free to go ahead merging otherwise

This implements a generic fix to use `repr()` for data to be encoded to JSON values that would normally cause errors like:
`TypeError: Object of type datetime is not JSON serializable`

Signed-off-by: Tobias Wolf <[email protected]>
@boekhorstb1 boekhorstb1 marked this pull request as ready for review September 23, 2024 07:16
@boekhorstb1 boekhorstb1 merged commit ad6f0da into main Sep 23, 2024
8 checks passed
@boekhorstb1 boekhorstb1 deleted the prs/adding-pickle-output-to-cli branch September 23, 2024 07:45
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.

2 participants