Repository navigation
Improve chatops messages - #285
Conversation
2eb4eef to
76665da
Compare
| } | ||
|
|
||
| conn.PostMessage(http.StatusAccepted, job.Name, res) | ||
| conn.PostMessage(http.StatusCreated, job.Name, res) |
There was a problem hiding this comment.
What does it look like to a user?
After this message a job gets Marked as done, so any positive HTTP status could would suffice. I do not see any reason to prefer StatusAccepted over StatusCreated.
I do not even understand why we would prefer a message on creation than a message on finishing.
There was a problem hiding this comment.
The way the current chatops implementation works is messages are based off the status code, so we need to use a different status code from the existing 'job accepted' state if we want a different message.
Since this message is a 'job done' state I went with 201 Created, but as long as its not 202 any 2XX would work.
Messages before changes:
r10k for Puppet
r10k deployment of Puppet environment XXX
Successfully started deployment of XXX
---
r10k for Puppet
r10k deployment of Puppet environment XXX
Successfully started deployment of XXX
Messages after changes:
r10k for Puppet on examplehost
r10k deployment of Puppet environment XXX
Successfully started deployment of XXX
---
r10k for Puppet on examplehost
r10k deployment of Puppet environment XXX
Successfully deployed XXX
There was a problem hiding this comment.
Would like to add the meaning of HTTP status codes:
Accepted is supposed to signal "creation of resource has begun" and Created is supposed to signal "creation of resource finished".
* Include hostname * Print a different message on success vs queued
e016fd8
76665da to
e016fd8
Compare
Currently you get the same message twice if you're using queues, adjust the second message to actually state the job is done.
Additionally add the hostname to the author so we can see which server has been deployed to.