`inspect`: confusion around the `message` parameter

[soxofaan]

• in job logs (GET /jobs/{jobid}/logs) the message field is required and the data field is optional
• in the proposed inspect process it's currently the other way around: data is required (first) argument and message is optional (even last) argument

I think there is a bit of a mismatch here.

[m-mohr]

Yes, it's a mismatch but still works. The API only requires a string of any length, so the default value of an empty string is valid (but not very useful).

Some changes we could make:

1. Make message the second parameter, still optional. Maybe add a different default message such as inspect data or so.
2. Make message the first parameter, required. Make data the second parameter, optional, with the default value null. In this case, it might be an issue that it's unclear whether data was not given or was null.

As data inspection is the primary use case for this process (I think), I'd slightly tend towards option 1.

[soxofaan]

As data inspection is the primary use case for this process (I think), I'd slightly tend towards option 1.

I agree. The least we can do is making message the second parameter instead of last.

Changing the default message to something like "inspect data" instead of empty string is also a good idea, to avoid empty entries in simple log views

[m-mohr]

message is now the second parameter.

I'm not sure about the default message yet. Having it might actually be annoying if you log a lot.

[soxofaan]

Having it might actually be annoying if you log a lot.

I think a minimal message is better than an empty string (especially if you have a compact log widget that only shows the message). It is a required field under GET /jobs/{jobid}/logs, so an empty string is a bit of lame workaround.

And if users find it annoying: that's an incentive to write a more descriptive one :smile: