Issue 5837051: Fix `juju status` bug when working with multiple relations for a service.
DescriptionFix `juju status` bug when working with multiple relations for a service.
Fix a bug in `juju status` status for the scenario that a service
has multiple established relations to consuming clients, each
using the same relation name from the provider's perspective. This
fix does introduce a backwards breaking change.
https://code.launchpad.net/~jimbaker/juju/juju-status-changes-spec/+merge/97738
(do not edit description out of merge proposal)
Patch Set 1 #
Total comments: 7
Patch Set 2 : Fix `juju status` bug when working with multiple relations for a service. #
Total comments: 2
MessagesTotal messages: 11
Please take a look.
Sign in to reply to this message.
https://codereview.appspot.com/5837051/diff/1/source/drafts/juju-status-chang... File source/drafts/juju-status-changes.rst (right): https://codereview.appspot.com/5837051/diff/1/source/drafts/juju-status-chang... source/drafts/juju-status-changes.rst:17: - {name: db, service: blog2} Rather than having a list of dictionaries, I suggest a dictionary of lists: relations: db: [blog1, blog2] https://codereview.appspot.com/5837051/diff/1/source/drafts/juju-status-chang... source/drafts/juju-status-changes.rst:24: - {name: db, service: blog2, state: down} I wonder about this one, though. Showing a "down" relation here is not very useful. We already have the information of which relations exist at the service level. I suggest we display information in the same format as for the service level, but only ever show those relations that are up. So, for the above example: relations: db: [blog1]
Sign in to reply to this message.
https://codereview.appspot.com/5837051/diff/1/source/drafts/juju-status-chang... File source/drafts/juju-status-changes.rst (right): https://codereview.appspot.com/5837051/diff/1/source/drafts/juju-status-chang... source/drafts/juju-status-changes.rst:17: - {name: db, service: blog2} On 2012/03/15 21:28:00, niemeyer wrote: > Rather than having a list of dictionaries, I suggest a dictionary of lists: > > relations: > db: [blog1, blog2] much nicer and succinct https://codereview.appspot.com/5837051/diff/1/source/drafts/juju-status-chang... source/drafts/juju-status-changes.rst:24: - {name: db, service: blog2, state: down} Its useful i think to explicitly state the instance of a particular relation is down rather than relying on a user inferring against the absence of a value that there is a problem. On 2012/03/15 21:28:00, niemeyer wrote: > I wonder about this one, though. Showing a "down" relation here is not very > useful. We already have the information of which relations exist at the service > level. I suggest we display information in the same format as for the service > level, but only ever show those relations that are up. So, for the above > example: > > relations: > db: [blog1]
Sign in to reply to this message.
https://codereview.appspot.com/5837051/diff/1/source/drafts/juju-status-chang... File source/drafts/juju-status-changes.rst (right): https://codereview.appspot.com/5837051/diff/1/source/drafts/juju-status-chang... source/drafts/juju-status-changes.rst:24: - {name: db, service: blog2, state: down} On 2012/03/16 01:30:59, hazmat wrote: > Its useful i think to explicitly state the instance of a particular relation is > down rather than relying on a user inferring against the absence of a value > that there is a problem. If the goal is to make things clear and obvious, having a massive amount of information for each relation won't help. What about splitting up, then: relations: db: [blog1] relations-pending: db: [blog2]
Sign in to reply to this message.
On 2012/03/16 02:53:42, niemeyer wrote: > https://codereview.appspot.com/5837051/diff/1/source/drafts/juju-status-chang... > File source/drafts/juju-status-changes.rst (right): > > https://codereview.appspot.com/5837051/diff/1/source/drafts/juju-status-chang... > source/drafts/juju-status-changes.rst:24: - {name: db, service: blog2, state: > down} > On 2012/03/16 01:30:59, hazmat wrote: > > Its useful i think to explicitly state the instance of a particular relation > is > > down rather than relying on a user inferring against the absence of a value > > that there is a problem. > > If the goal is to make things clear and obvious, having a massive amount of > information for each relation won't help. > > What about splitting up, then: > > relations: > db: [blog1] > relations-pending: > db: [blog2] that sounds reasonable.. it would be relations-down.. pending isn't valid from the unit's perspective, it either knows about and its up or down, i mean we could infer, but its a very transient state, effectively the time between the add-relation and a watch firing. also i wonder if it makes sense to only use list format for relations that are server oriented, for client relations its only a singleton by name. it does add minor ambiguity to the parsing as interface role isn't available in status. we should probably run this by the list before it lands, as its going to break the extant status parsers.
Sign in to reply to this message.
On Fri, Mar 16, 2012 at 00:03, <kapilt@gmail.com> wrote: > that sounds reasonable.. it would be relations-down.. pending isn't > valid from the unit's perspective, it either knows about and its up or > down, i mean we could infer, but its a very transient state, effectively > the time between the add-relation and a watch firing. So what's a down relation, and why does it matter? My understanding is that while the agent doesn't recognize the relation it's down. For a user, "pending" feels like a more understandable term for that. > also i wonder if it makes sense to only use list format for relations > that are server oriented, for client relations its only a singleton by > name. it does add minor ambiguity to the parsing as interface role isn't > available in status. we should probably run this by the list before it > lands, as its going to break the extant status parsers. I think we over that quite a few times already. :-) Both requires and provides may have multiple endpoints. -- Gustavo Niemeyer http://niemeyer.net http://niemeyer.net/plus http://niemeyer.net/twitter http://niemeyer.net/blog -- I'm not absolutely sure of anything.
Sign in to reply to this message.
On Thu, Mar 15, 2012 at 11:19 PM, Gustavo Niemeyer < gustavo.niemeyer@canonical.com> wrote: > On Fri, Mar 16, 2012 at 00:03, <kapilt@gmail.com> wrote: > > > that sounds reasonable.. it would be relations-down.. pending isn't > > valid from the unit's perspective, it either knows about and its up or > > down, i mean we could infer, but its a very transient state, effectively > > the time between the add-relation and a watch firing. > > So what's a down relation, and why does it matter? My understanding is > that while the agent doesn't recognize the relation it's down. For a > user, "pending" feels like a more understandable term for that. > down represents an error state that needs user resolution. > I think we over that quite a few times already. :-) Both requires and > provides may have multiple endpoints. > ack.
Sign in to reply to this message.
On Fri, Mar 16, 2012 at 10:24, Kapil Thangavelu <kapilt@gmail.com> wrote: > down represents an error state that needs user resolution. Can you please expand a bit on that? We already have an error state saying that a unit has problems. What's the benefit/reason why we tag a specific relation as down? -- Gustavo Niemeyer http://niemeyer.net http://niemeyer.net/plus http://niemeyer.net/twitter http://niemeyer.net/blog -- I'm not absolutely sure of anything.
Sign in to reply to this message.
Please take a look. https://codereview.appspot.com/5837051/diff/1/source/drafts/juju-status-chang... File source/drafts/juju-status-changes.rst (right): https://codereview.appspot.com/5837051/diff/1/source/drafts/juju-status-chang... source/drafts/juju-status-changes.rst:17: - {name: db, service: blog2} On 2012/03/15 21:28:00, niemeyer wrote: > Rather than having a list of dictionaries, I suggest a dictionary of lists: > > relations: > db: [blog1, blog2] Done. https://codereview.appspot.com/5837051/diff/1/source/drafts/juju-status-chang... source/drafts/juju-status-changes.rst:24: - {name: db, service: blog2, state: down} Updated, along with a generalization to show relations-<state name> if nonempty.
Sign in to reply to this message.
https://codereview.appspot.com/5837051/diff/9001/source/drafts/juju-status-ch... File source/drafts/juju-status-changes.rst (right): https://codereview.appspot.com/5837051/diff/9001/source/drafts/juju-status-ch... source/drafts/juju-status-changes.rst:41: db: [blog2] This doesn't reflect the latest agreement. Please check out the email from Kapil to the list. I actually wonder.. do we need this spec now? It was good to spark the debate and reach an agreement, but I'm personally happy with Kapil's email alone now.
Sign in to reply to this message.
https://codereview.appspot.com/5837051/diff/9001/source/drafts/juju-status-ch... File source/drafts/juju-status-changes.rst (right): https://codereview.appspot.com/5837051/diff/9001/source/drafts/juju-status-ch... source/drafts/juju-status-changes.rst:41: db: [blog2] Weird, my email client (Thunderbird) had not synced this Friday email sometime this morning, but it's there now. I'm happy with Kapil's email as well. On 2012/03/19 22:23:14, niemeyer wrote: > This doesn't reflect the latest agreement. Please check out the email from Kapil > to the list. > > I actually wonder.. do we need this spec now? It was good to spark the debate > and reach an agreement, but I'm personally happy with Kapil's email alone now.
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
