Skip to content

Conversation

allmightyspiff
Copy link
Member

#1100

Changes to the slcli event-log get command to better support pagination.

Since this command takes 20-40s to return, I went with directly outputting the results with a generator so users can constantly see updates if they request a lot of items. Sadly PrettyTable doesn't really support that behavior.

@allmightyspiff allmightyspiff added CLI Core Issues dealing with core functionality labels Mar 27, 2019
@allmightyspiff allmightyspiff self-assigned this Mar 27, 2019
@coveralls
Copy link

coveralls commented Mar 28, 2019

Coverage Status

Coverage decreased (-0.05%) to 91.775% when pulling 018400e on allmightyspiff:issues1100 into 0c21f70 on softlayer:master.

@allmightyspiff
Copy link
Member Author

100% coverage on event_log, so I'm going to ignore the -0.06% coverage decrease, mostly due to there being less lines overall.

SoftLayer/CLI/event_log/__init__.py                                                                          0      0   100%
SoftLayer/CLI/event_log/get.py                                                                              44      0   100%
SoftLayer/CLI/event_log/types.py                                                                            13      0   100%
SoftLayer/managers/event_log.py                                                                             30      0   100%

@allmightyspiff
Copy link
Member Author

Biggest change here is support for limit in the slcli event-log get command, and the change of output format. Since this command take a long time to resolve, I decided to make heavy use of the python generator from iter_call and just print out results as we get them. Sadly pretty_table doesn't seem to work with just iteratively printing out a table like this.

Copy link
Contributor

@acamacho82 acamacho82 left a comment

Choose a reason for hiding this comment

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

looks good, but I found that --limit=-1 prints only 1 event instead of ALL events

Copy link
Contributor

@ATGE ATGE left a comment

Choose a reason for hiding this comment

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

looks good

Copy link
Contributor

@FernandoOjeda FernandoOjeda left a comment

Choose a reason for hiding this comment

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

It looks good.

@allmightyspiff allmightyspiff merged commit cfab07e into softlayer:master Apr 8, 2019
@allmightyspiff allmightyspiff deleted the issues1100 branch August 31, 2020 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Core Issues dealing with core functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants