Nir Soffer has posted comments on this change. Change subject: vdsm: Updated documentation for vdsClient's downloadImage method ......................................................................
Patch Set 1: (12 comments) Please display the new help text in the shell and copy and paste the result here so we can evaluate this patch without checking it out and trying it. http://gerrit.ovirt.org/#/c/38010/1/client/vdsClient.py File client/vdsClient.py: Line 2589 Line 2590 Line 2591 Line 2592 Line 2593 Note: from this point, we are dealing with one string, not list of strings. Line 2591: )), Line 2592: 'downloadImage': (serv.downloadImage, ( Line 2593: '<methodArgs> <spUUID> <sdUUID> <imgUUID> [<volUUID>]', Line 2594: 'Download an image from a remote endpoint using the specified ' Line 2595: 'methodArgs.', Please remove all the "," at the end of the lines on this and next line. Line 2596: '', Line 2597: 'Arguments:', Line 2598: '@methodArgs: Python dictionary literal specifying the download method.', Line 2599: ' The dictionary must be quoted (e.g, \'{"methodArgs": {"url": ...}\')', Line 2594: 'Download an image from a remote endpoint using the specified ' Line 2595: 'methodArgs.', Line 2596: '', Line 2597: 'Arguments:', Line 2598: '@methodArgs: Python dictionary literal specifying the download method.', We don't need to use the schema syntax here. Just use the name of the variable. But showing the argument as a list can be nice: Arguments: - item description - item description Or we can use indentation: Arguments: item description item description The last option is more common. Line 2599: ' The dictionary must be quoted (e.g, \'{"methodArgs": {"url": ...}\')', Line 2600: ' Keys:', Line 2601: ' - url: url of the source image', Line 2602: ' - headers: dictionary of optional headers', Line 2598: '@methodArgs: Python dictionary literal specifying the download method.', Line 2599: ' The dictionary must be quoted (e.g, \'{"methodArgs": {"url": ...}\')', Line 2600: ' Keys:', Line 2601: ' - url: url of the source image', Line 2602: ' - headers: dictionary of optional headers', Please check if the headers dictionary is optional or it must be specified as empty dictionary and document this here. Line 2603: ' - method: currently only "http" is supported', Line 2604: '@spUUID: The UUID of the Storage Pool associated with the Image', Line 2605: '@sdUUID: The UUID of the Storage Domain associated with the Image', Line 2606: '@imgUUID: The UUID of the Image', Line 2603: ' - method: currently only "http" is supported', Line 2604: '@spUUID: The UUID of the Storage Pool associated with the Image', Line 2605: '@sdUUID: The UUID of the Storage Domain associated with the Image', Line 2606: '@imgUUID: The UUID of the Image', Line 2607: '@volUUID: #optional The UUID of the Volume', No need for schema syntax here (#optional) Line 2608: '', Line 2609: 'Example:', Line 2610: 'vdsClient -s 0 downloadImage ' Line 2611: '{"url": "http://example.com/myimage", "headers": {"Foo": "Bar"}, "method": "http"} \ ', Line 2605: '@sdUUID: The UUID of the Storage Domain associated with the Image', Line 2606: '@imgUUID: The UUID of the Image', Line 2607: '@volUUID: #optional The UUID of the Volume', Line 2608: '', Line 2609: 'Example:', Examples should be last. Line 2610: 'vdsClient -s 0 downloadImage ' Line 2611: '{"url": "http://example.com/myimage", "headers": {"Foo": "Bar"}, "method": "http"} \ ', Line 2612: 'spUUID sdUUID imgUUID', Line 2613: '', Line 2606: '@imgUUID: The UUID of the Image', Line 2607: '@volUUID: #optional The UUID of the Volume', Line 2608: '', Line 2609: 'Example:', Line 2610: 'vdsClient -s 0 downloadImage ' This line must be terminated by "\" so the user can copy and paste it to the shell. Line 2611: '{"url": "http://example.com/myimage", "headers": {"Foo": "Bar"}, "method": "http"} \ ', Line 2612: 'spUUID sdUUID imgUUID', Line 2613: '', Line 2614: 'Returns:', http://gerrit.ovirt.org/#/c/38010/1/vdsm/rpc/vdsmapi-schema.json File vdsm/rpc/vdsmapi-schema.json: Line 4657: # Line 4658: # Download an image to a remote endpoint using the specified method Line 4659: # and methodArgs. Line 4660: # Line 4661: # Arguments: Not needed in the schema - look at other method definitions. Line 4662: # Line 4663: # @methodArgs: Python dictionary literal specifying the download method. Line 4664: # The dictionary must be quoted (e.g, '{"methodArgs": {"url": ...}') Line 4665: # Keys: Line 4659: # and methodArgs. Line 4660: # Line 4661: # Arguments: Line 4662: # Line 4663: # @methodArgs: Python dictionary literal specifying the download method. > In this file, the context is different than for vdsClient in that we're des Using ImageSharingMethodArgs would be fine. We should replace it with a simpler and clear type later. Line 4664: # The dictionary must be quoted (e.g, '{"methodArgs": {"url": ...}') Line 4665: # Keys: Line 4666: # - url: url of the source image Line 4667: # - headers: dictionary of optional headers Line 4660: # Line 4661: # Arguments: Line 4662: # Line 4663: # @methodArgs: Python dictionary literal specifying the download method. Line 4664: # The dictionary must be quoted (e.g, '{"methodArgs": {"url": ...}') This is not relevant to the api. quoting is an issue only in the vdsClient command line. Line 4665: # Keys: Line 4666: # - url: url of the source image Line 4667: # - headers: dictionary of optional headers Line 4668: # - method: currently only "http" is supported Line 4664: # The dictionary must be quoted (e.g, '{"methodArgs": {"url": ...}') Line 4665: # Keys: Line 4666: # - url: url of the source image Line 4667: # - headers: dictionary of optional headers Line 4668: # - method: currently only "http" is supported > Please remove the trailing whitespace. Not needed here. Line 4669: # Line 4670: # @spUUID: The UUID of the Storage Pool associated with the Image Line 4671: # Line 4672: # @sdUUID: The UUID of the Storage Domain associated with the Image Line 4676: # @volUUID: #optional The UUID of the Volume Line 4677: # Line 4678: # Example: Line 4679: # Line 4680: # vdsClient -s 0 downloadImage \ > Last note on context: the API example would be different than vdsClient, bu We don't need the example here Line 4681: # {"url": "http://example.com/myimage", "headers": {"Foo": "Bar"}, "method": "http"}' \ Line 4682: # spUUID sdUUID imgUUID Line 4683: # Line 4684: # Returns: -- To view, visit http://gerrit.ovirt.org/38010 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3a13e7a46bbb2e847ea6c5bd7a1315a0a4de2fc5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Candace Sheremeta <csher...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Amit Aviram <aavi...@redhat.com> Gerrit-Reviewer: Candace Sheremeta <csher...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Fred Rolland <froll...@redhat.com> Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com> Gerrit-Reviewer: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches