Change in vdsm[master]: vdsm: Updated documentation for vdsClient's downloadImage me...

nsoffer at redhat.com nsoffer at redhat.com
Sat Feb 21 00:38:52 UTC 2015


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 <cshereme at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Amit Aviram <aaviram at redhat.com>
Gerrit-Reviewer: Candace Sheremeta <cshereme at redhat.com>
Gerrit-Reviewer: Daniel Erez <derez at redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland at redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett at redhat.com>
Gerrit-Reviewer: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Liron Aravot <laravot at redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipchuk at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: Vered Volansky <vvolansk at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list