Refactor modify_config method #87
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
Add test for checking if a deprecation warning is raised when passing a string as alert_ids to create_task method.
modify_config supports at least four different modes via the selection argument and the availibility of different parameters. This can't be mapped very well to a single function/method and therefore new more specific modify_config_set_ methods are introduced for the different use cases.
sphinx interpreted the modify_config_set_ string as a reference to another section or link.
Codecov Report
@@ Coverage Diff @@
## master #87 +/- ##
===========================================
+ Coverage 77.16% 88.01% +10.84%
===========================================
Files 10 10
Lines 2308 2377 +69
===========================================
+ Hits 1781 2092 +311
+ Misses 527 285 -242
Continue to review full report at Codecov.
|
wiegandm
previously approved these changes
Dec 24, 2018
Also don't add credential_type to the modify_credential request. It seems the protocol doesn't allow for the type parameter.
Handle allow_insecure argument of modify_credential as a boolean
Don't ignore them silently if an empty string is passed. Either set the passed value as empty in the request or raise an error.
The logic in modify_credential could be improved if it would be validated against the implemented protocol in gvmd. Currently it follows the spec only mostly.
* port argument should be an int * threat, new_threat should be one of High, Medium, Low, Alarm, Log, Debug * severity, new_severity should be a decimal value * hosts will be joined without an extra space
Both params are mentioned in the protocol docs but aren't in the docs for the modify commands and also aren't available in gsad. Therefore remove them from create commands.
Use same checks as create_permission
The exception is raised within import_report method
The comment of a report isn't used (at least not in gsa) so there is no need to update it.
Add param element if param_name is passed. For me it doesn't make much sense to add <param><name> without <value> but the protocol spec seems to allow this.
It seems to be ignored by gvmd.
If user_id is present use user_id instead of name for user modification.
Require either setting_id or name not both. Allow to reset a value by passing an empty string as value argument.
wiegandm
approved these changes
Dec 28, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Split modify_config method into several distinct methods for each use case.