Michael Pasternak has posted comments on this change.
Change subject: cli: add the option to retrieve partial history using
CLI(#957499)
......................................................................
Patch Set 1: Code-Review-1
(5 comments)
....................................................
File src/ovirtcli/command/history.py
Line 71:
Line 72: def printHistoryAtIndex(self, args):
Line 73: context = self.context
Line 74: indx = args[0]
Line 75: history = context.history.list()
why this ^ is needed? you print history at given position here ...
Line 76: if history:
Line 77: try:
Line 78: slide = int(indx)
Line 79: h_item = context.history.get(slide)
Line 93: val = '';
Line 94: for key in options.keys():
Line 95: prop = key.replace('--', '')
Line 96: val = options[key]
Line 97:
you don't need to loop?, you should be fetching value in o(1) from map here by
keys --last/--first
Line 98: history = context.history.list()
Line 99: if history:
Line 100: history_len = len(history)
Line 101: slide_len = int(val)
Line 100: history_len = len(history)
Line 101: slide_len = int(val)
Line 102: if prop == 'last':
Line 103: self.printHistoryBetweenIndexes(max(0, history_len -
slide_len), history_len)
Line 104: elif prop == 'first':
i can't see you exposing --fisrt/--last options for auto-completion, they won't
be visible to user this way
Line 105: self.printHistoryBetweenIndexes(0, min(slide_len,
history_len))
Line 106: else:
Line 107: self.error('Unknown property ' + prop)
Line 108:
Line 103: self.printHistoryBetweenIndexes(max(0, history_len -
slide_len), history_len)
Line 104: elif prop == 'first':
Line 105: self.printHistoryBetweenIndexes(0, min(slide_len,
history_len))
Line 106: else:
Line 107: self.error('Unknown property ' + prop)
unknown options should be silently ignored
Line 108:
Line 109: def printHistoryBetweenIndexes(self, from_index, to_index):
Line 110: context = self.context
Line 111: try:
Line 105: self.printHistoryBetweenIndexes(0, min(slide_len,
history_len))
Line 106: else:
Line 107: self.error('Unknown property ' + prop)
Line 108:
Line 109: def printHistoryBetweenIndexes(self, from_index, to_index):
i'd add this ^ functionality to HistoryManager#list => at .history.list(...)
Line 110: context = self.context
Line 111: try:
Line 112: self.write('')
Line 113: for slide in range(from_index, to_index):
--
To view, visit http://gerrit.ovirt.org/20272
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2321ccc7e3cce2521e0b86f67de2797a8a2b4276
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine-cli
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
