ios_logging: Fix some smaller issues, add unit test #32321
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ansibot
added
affects_2.5
|
The test The test |
ansibot
added
needs_revision
|
@paneu This is great, thank you. Will review and look at backporting to |
gundalow
removed
the
needs_triage
trishnaguha
reviewed
Oct 30, 2017
| @@ -47,7 +47,8 @@ | |||
| size: | |||
| description: | |||
| - Size of buffer. The acceptable value is in range from 4096 to | |||
| 4294967295 bytes. | |||
| 4294967295 bytes. If value of C(dest) is I(buffered), the default value | |||
| is 4096. | |||
There was a problem hiding this comment.
you can have a key named default instead.
This should look like the following:
size:
description:
- Size of buffer. The acceptable value is in range from 4096 to
4294967295 bytes.
default: 4096
trishnaguha
suggested changes
Oct 30, 2017
| self.execute_module(changed=True, commands=commands) | ||
|
|
||
| def test_ios_logging_buffer_size_changed_explicit(self): | ||
| set_module_args(dict(dest='buffered', size=4096)) |
There was a problem hiding this comment.
A different input is suggested for the size for the explicit test here to make sure it works as expected.
ansibot
added
needs_revision
trishnaguha
suggested changes
Oct 30, 2017
| @@ -297,7 +298,7 @@ def map_params_to_obj(module, required_if=None): | |||
|
|
|||
| if module.params['dest'] == 'buffered': | |||
| if not module.params['size']: | |||
| module.params['size'] = str(4096) | |||
| module.params['size'] = int(4096) | |||
There was a problem hiding this comment.
Converting the value to int here would cause idempotence issue.
Rather you can convert the value to int inside the function validate_size itself to make the default value work.
This would be the consistent way to do it:
def validate_size(value, module):
if value:
- if not int(4096) <= value <= int(4294967295):
+ if not int(4096) <= int(value) <= int(4294967295):
…size When the size parameter is not configured while configuring the buffered destination, a traceback occurs due to the fact that validate_size expects the parameter to be an int. Explicitely converting value to int makes the check work for every case.
Update the documentation of the size paramter to reflect the current behaviour of setting a default of 4096 for the buffered dest.
Add unit test for ios_logging testing the behaviour clarified in the previous commits.
|
I have tried to incorporate your feedback. Please let me know whether there are still issues with the code. |
trishnaguha
approved these changes
Oct 31, 2017
|
Cherry-picked to stable-2.4 for 2.4.2beta2 |
abadger
pushed a commit
that referenced
this pull request
Nov 1, 2017
* ios_logging: Fix typo in documentation * ios_logging: Fix traceback when setting buffered destination without size When the size parameter is not configured while configuring the buffered destination, a traceback occurs due to the fact that validate_size expects the parameter to be an int. Explicitely converting value to int makes the check work for every case. * ios_logging: Update size parameter documentation Update the documentation of the size paramter to reflect the current behaviour of setting a default of 4096 for the buffered dest. * ios_logging: Add unit test Add unit test for ios_logging testing the behaviour clarified in the previous commits. * ios_logging: Fix python 2.6 compliance (cherry picked from commit 53fead7)
ansibot
added
bug
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
ISSUE TYPE
COMPONENT NAME
ios_logging
ADDITIONAL INFORMATION
Fix smaller issues with ios_logging (e.g. documentation). Also add a unit test for testing for the bug fixed.