16307 - Restore ability for reports to be able to log_success without generating a message

[Julio-Oliveira-Encora]

Fixes: #16307

It was added to bypass the log message when the status is success, and the message is None.

It was tested using log_success(None, None), log_success("", None), and log_sucess().

[arthanson]

@Julio-Oliveira-Encora it looks like log_success() works now with this change, can you please add a note to the documentation on scripts about this.

[jeremystretch]

@Julio-Oliveira-Encora have you tested this before submitting the PR? You haven't actually updated the method's signature to make message an optional argument. When calling log_success() in a script, an exception is raised:

Traceback (most recent call last):
  File "/home/jstretch/projects/netbox/netbox/extras/scripts.py", line 658, in _run_script
    script.output = script.run(data, commit)
  File "/home/jstretch/projects/netbox/netbox/scripts/test2.py", line 19, in run
    self.log_success()
TypeError: BaseScript.log_success() missing 1 required positional argument: 'message'

[Julio-Oliveira-Encora]

I tested it before submitting the PR. I will check the method's signature and test again.

[jeremystretch]

I'm unclear on what the intended goal is here. As it stands, calling log_success(obj=thing) in a custom script does not generate any output. This seems completely pointless, and confusing.

#16307 seems limited to incrementing success counters in reports only; if that's the case, the log_*() methods on the Report class should be modified to call self._log() directly rather than proxying through the log_*() methods on Script, to avoid changing them.

[Julio-Oliveira-Encora]

@jeremystretch About the log methods on the Report class, should I change them this way?

def log_success(self, obj=None, message=None):
     self._log(message, obj, level=LogLevelChoices.LOG_SUCCESS)

Or follow the argument positional from scripts, where message is before obj?

def log_success(self, message, obj=None):
     self._log(message, obj, level=LogLevelChoices.LOG_SUCCESS)

Should I extend this change for all log methods in the class (log_info, log_warning...)?

[Julio-Oliveira-Encora]

We could discuss it more because the user is asking for the change for Reports, but maybe he wants to use it in all types of scripts.

[jeremystretch]

@Julio-Oliveira-Encora I think we went down the wrong path with this a bit. I've submitted an alternate PR (#16697) which I believe addresses the proposal in #16307.