search

arista-eosplus / rbeapi

Ruby client for Arista eAPI
BSD 3-Clause "New" or "Revised" License
16 stars 16 forks source link

(maint) Enhance configure exception info #168

Closed shermdog closed 7 years ago

shermdog commented 7 years ago

Previously configure would only return false on failure, which is not overly useful. This commit includes the exception itself

Additionally CommandError now includes the err from the device.

This makes output in higher level tools far more actionable:

Notice: /Stage[main]/Main/Ntp_server[0.us.pool.ntp.org]/source_interface: defined 'source_interface' as 'Management1'
Notice: /Stage[main]/Main/Ntp_server[0.us.pool.ntp.org]/vrf: defined 'vrf' as 'test'
Error: CLI command 3 of 3 'ntp server vrf test 0.us.pool.ntp.org prefer source Management1 ' failed: could not run command
All NTP servers must be in the same VRF. Please remove time servers from VRF default before continuing.
eosplus-jenkins commented 7 years ago

Can one of the admins verify this patch?

shermdog commented 7 years ago

@jerearista This could have some backwards compat issues if folks are checking the output == false, but IMO, this is far more valuable.

jerearista commented 7 years ago

Jenkins, test this please

shermdog commented 7 years ago

@jerearista How did jenkins feel about this?

jerearista commented 7 years ago

This impacts the following:

I will verify against our aristanetworks/eos module, too.

For the record, this is a really good enhancement once we make sure we don't negatively impact anything else.

jerearista commented 7 years ago

https://github.com/arista-eosplus/puppet-eos module tests pass with this change

shermdog commented 7 years ago

Closing for now. Agreed to move this to 2.0