Add a timeout argument to the junos_commit module

[stacywsmith]

Add a timeout argument to the junos_commit module to support confirming a commit on a device which is slow and/or has a large config.

[vnitinv]

@stacywsmith Can we change the logic where we can simply have 30 sec default value @ timeout=dict(required=False, default=0),
hence the code would change to

        timeout = int(args['timeout']) 
        dev.timeout = timeout

[stacywsmith]

@vnitinv Hi Nitin the code in my pull request was copied from the implementation of the timeout argument that's currently used for junos_install_config. I believe you may have misinterpreted the behavior of the code.

If the user does not pass the timeout argument, then line 153 will set timeout = 0, line 154 is not true, and dev.timeout will not be set.

If the user passes a timeout value, then line 153 sets timeout to that value (rounded to nearest int). If the value the user passed is greater than 0, line 154 is true and dev.timeout is set to the user supplied value.

Therefore, if a user passes any numeric value greater than 0, the RPC timeout will be set to the user supplied value.

Here's an example from the debugger.

'''
(Epdb) print args
{'comment': None, 'host': 'core1.site1', 'user': 'user', 'timeout': 10, 'confirm': None, 'passwd': 'user123', 'logfile': None, 'port': 830}
(Epdb) n

/home/user/.ansible/tmp/ansible-tmp-1449948726.22-132107123925044/junos_commit(156)main()
-> timeout = int(args['timeout'])
(Epdb) n
/home/user/.ansible/tmp/ansible-tmp-1449948726.22-132107123925044/junos_commit(157)main()
-> if timeout > 0:
(Epdb) timeout
10
(Epdb) n
/home/user/.ansible/tmp/ansible-tmp-1449948726.22-132107123925044/junos_commit(158)main()
-> dev.timeout = timeout
(Epdb) n
/home/user/.ansible/tmp/ansible-tmp-1449948726.22-132107123925044/junos_commit(160)main()
-> cu = Config(dev)
(Epdb) dev.timeout
10
(Epdb)
'''

You're suggested change is similar in behavior, but would allow the user to set the timeout to a negative value. This causes PyEZ to raise a jnpr.junos.exception.LockError exception which in turn causes the Ansible task/module to fail. Worse yet, it leaves the configuration databases locked causing all further attempts to lock the database to fail.

I believe my existing pull request (copied from the junos_install_config implementation) is preferrable.

[vnitinv]

Thanks @stacywsmith
Can you check line https://github.com/Juniper/ansible-junos-stdlib/blob/master/library/junos_install_os#L234
to predefine timeout data type. with this code would change to
timeout = intargs['timeout']

[stacywsmith]

Are you suggesting changing line 120 from:
''' timeout=dict(required=False, default=0), '''

to:
''' timeout=dict(required=False, type='int', default=0), '''

If so, I'm fine with that change. However, we should also change this line to match:
https://github.com/Juniper/ansible-junos-stdlib/blob/master/library/junos_install_config#L399

Even with this change, lines 153-155 need to be:
'''
timeout = args['timeout']
if timeout > 0:
dev.timeout = timeout
'''

I'm not sure if your suggesting this, but I don't support changing line 120 to:
''' timeout=dict(required=False, type='int', default=30), '''

I believe the default RPC timeout should continue to be set in only one location (in PyEZ), rather than in both PyEZ (and within multiple locations in the Ansible for Junos modules).

[vnitinv]

Hi Stacy,

I just meant it to be

timeout=dict(required=False, type='int', default=0)