Added support for retrievel of firmware from an FTP server

[andy-maier]

For details, see the commit message.
This PR has not been tested so far.

To try this version, install it from its branch:

pip install zhmccli@git+https://github.com/zhmcclient/zhmccli.git@andy/upgrade-ftp-retrieval

[coveralls]

coverage: 37.054% (-0.09%) from 37.143%
when pulling 67f82f2 on andy/upgrade-ftp-retrieval
into 4fc2a3a on master.

[EdGue42]

code duplication? could the two ifs go into a utility method?

[andy-maier]

I assume this comment is about the one "if" 5 lines down from the location you commented on, and the same "if" in the console module:

    if ftp_host:
        kwargs['ftp_host'] = ftp_host
        kwargs['ftp_protocol'] = options['ftp_protocol']
        kwargs['ftp_user'] = ftp_user
        kwargs['ftp_password'] = ftp_password
        kwargs['ftp_directory'] = options['ftp_directory']

I'm not enthusiastic about introducing a cross-module utility function in the helper module just for these 5 updates. It makes it more difficult to see what is going on.

Also, if you suspect a maintenance problem because these properties might change, then what makes you think that they could not change differently between HMC and CPC?

Finally, with 5 rather similar input parameters, the utility function would likely be invoked with keyword args to make sure there is no unintentional misordering of positional parameters, so the 6 lines shown above would become something like this:

    kwargs.update(ftp_kwargs(
        ftp_host=ftp_host,
        ftp_protocol=options['ftp_protocol'],
        ftp_user=ftp_user,
        ftp_password=ftp_password,
        ftp_directory=options['ftp_directory']))

And this hides the fact that the kwargs are only updated if ftp_host is set (which is what I meant when I said it is more difficult to see what is going on).

Also, even if a new parameter gets added to both the CPC and Console case in the future, both invocations need to be adjusted. The only small advantage of a utility function would be that the second adjustment could not be forgotten, if (!) the new parameter to the utility function has no default. And I'm not sure that's worth it.

PS: Would these 6 identical lines between the CPC and Console modules still count as a code duplication ? ;-)

[EdGue42]

Well, it is "duplicated code" if there is a realistic chance that future changes would affect all those "copies".
In the end, it is your call to make ;-)

[andy-maier]

The bundle-level parameter is now optional. Support for this needs to be added.

Update: DONE

[EdGue42]

Well, it is "duplicated code" if there is a realistic chance that future changes would affect all those "copies".
In the end, it is your call to make ;-)