Event Log improvements #1103
Event Log improvements #1103
Conversation
There was a problem hiding this comment.
Adding a limit here without adding the offset is a bit problematic since it makes certain records unobtainable.
My initial thought here would be to change the manager call to self.client.call('Event_Log', 'getAllObjects', iter=True, filter=request_filter, limit=log_limit) but I'm concerned that accounts with a large number of logs will get bogged down waiting for this command to finish.
I think just adding an offset and paginate option might be a good choice. If you can get the CLI to print out how many total entries there are (this is recorded in the request as total_items https://github.com/softlayer/softlayer-python/blob/master/SoftLayer/transports.py#L141 ) before iteration this might also be helpful.
|
|
||
| for log in logs: | ||
| user = log['userType'] | ||
| label = '' |
There was a problem hiding this comment.
label = log.get('label', '')
https://docs.python.org/3/library/stdtypes.html#dict.get
| @click.option('--metadata/--no-metadata', default=False, | ||
| help="Display metadata if present") | ||
| @click.option('--limit', '-l', default=30, | ||
| help="How many results to get in one api call, default is 30.") |
There was a problem hiding this comment.
https://click.palletsprojects.com/en/6.x/api/#click.Option
use show_default=True instead of adding it manually to the help message
| pass # label is already at default value. | ||
|
|
||
| if user == "CUSTOMER": | ||
| user = usrmgr.get_user(log['userId'], "mask[username]")['username'] |
There was a problem hiding this comment.
Just noticed in testing that this line generates a large number of redundant API calls. Would be worth cacheing the results so we only have to look up each user once. I don't want to change this to just get all users once because some accounts have a lot of users which would generate its own problems.
If you think thats out of the scope of this pull request thats ok, I can make an issue about it and deal with it later.
|
I'm going to resolve this in #1125 |
This adds limits and a couple fixes I found when playing with some other parts of the code.