mbruzek / docs

Juju documentation, found at http://juju.ubuntu.com/docs
1 stars 1 forks source link

merge reference-hook-tools with developer-hook-tools #15

Open mbruzek opened 8 years ago

mbruzek commented 8 years ago

These documents are very similar and the developer-hook-tools is more correct, and up to date.

evilnick commented 8 years ago

yeah, I completely disagree.

1) the hooks tools are a REFERENCE document, and should be in the reference section 2) although I guess you would want to update the examples (why can't we have bash and charmhelpers?), the explanations and caveats given in the reference version of the doc are actually more comprehensive in the majority of cases: e.g. compare:


opened-ports

opened-ports lists all ports or ranges opened by the unit.

from subprocess import check_output

range = check_output(["opened-ports"]) opened-ports opened-ports.exe


with:

opened-ports

The opened-ports hook tool lists all the ports currently opened by the running charm. It does not, at the moment, include ports which may be opened by other charms co-hosted on the same machine lp #1427770.

The command returns a list of one port or range of ports per line, with the port number followed by the protocol (tcp or udp).

For example, running opened-ports may return:

70-80/tcp 81/tcp !!! Note: opening ports is transactional (i.e. will take place on successfully exiting the current hook), and therefore opened-ports will not return any values for pending open-port operations run from within the same hook.


so, the only line of explanitory text in your preferred version is not only scanty, but also completely wrong, just in case you missed that.

lazypower commented 8 years ago

I think this is systemic of a larger issue that should be redirected at the juju/juju repository where the output text from juju help-tool is "completely wrong" - and should be addressed there.

The guide we have produced here, may be scant, but this is room for improvement as is anything that goes into the docs. We welcome your contributions - but the only added bits i see here is the blurb about transactional updates, and I disagree that it makes one doc vs the other more complete.

evilnick commented 8 years ago

1) I agree that the juju help items are, largely, not entirely useful. I think many devs would agree with that.

2) Yes, there is room for improvement - what I am trying to prevent is the wholesale deletion of useful information that took real people a real amount of time to create and verify from really actually using the software, which I don't view as an overall 'improvement' at all.

Are you honestly saying that replacing this:


status-set

The status mechanism allows Juju and its charms to accurately reflect their current status. This places the responsibility on the charm to know its status, and set it accordingly using the status-set hook tool.

This hook tool takes 2 arguments. The first is the status to report, which can be one of the following:

maintenance (the unit is not currently providing a service, but expects to be soon, E.g. when first installing) blocked (the unit cannot continue without user input) waiting (the unit itself is not in error and requires no intervention, but it is not currently in service as it depends on some external factor, e.g. a service to which it is related is not running) active (This unit believes it is correctly offering all the services it is primarily installed to provide) For more extensive explanations of these statuses, and other possible status values which may be set by Juju itself, please see the status reference page.

The second argument is a user-facing message, which will be displayed to any users viewing the status, and will also be visible in the status history. This can contain any useful information.

This status message provides valuable feedback to the user about what is happening. Changes in the status message are not broadcast to peers and counterpart units - they are for the benefit of humans only, so tools representing Juju services (e.g. the Juju GUI) should check occasionally and be told the current status message.

Spamming the status with many changes per second is therefore rather redundant (and might be throttled by the state server). Nevertheless, a thoughtful charm will provide appropriate and timely feedback for human users, with estimated times of completion of long-running status changes.

In the case of a blocked status though the status message should tell the user explicitly how to unblock the unit insofar as possible, as this is primary way of indicating any action to be taken (and may be surfaced by other tools using Juju, e.g. the Juju GUI).

A unit in the active state with should not generally expect anyone to look at its status message, and often it is better not to set one at all. In the event of a degradation of service, this is a good place to surface an explanation for the degradation (load, hardware failure or other issue).

A unit in error state will have a message that is set by Juju and not the charm because the error state represents a crash in a charm hook - an unmanaged and uninterpretable situation. Juju will set the message to be a reflection of the hook which crashed. For example “Crashed installing the software” for an install hook crash, or “Crash establishing database link” for a crash in a relationship hook.

Examples:

status-set maintenance "installing software" status-set maintenance "formatting storage space, time left: 120s" status-set waiting "waiting for database" status-set active status-set active "Storage 95% full" status-set blocked "Need a database relation" status-set blocked "Storage full"


with this:


status-set

status-set wites the workload state with an optional message which is visible to the user using juju status

from charmhelpers.core.hookenv import status_set

status_set('blocked', 'Unable to continue until related to a database') status-set blocked 'Unable to continue until related to a database' status-set.exe "active"


is an improvement?

lazypower commented 8 years ago

I do because its concise and essentially says the same thing.

for example, why is this even here?

A unit in error state will have a message that is set by Juju and not the charm because the error state represents a crash in a charm hook - an unmanaged and uninterpretable situation. Juju will set the message to be a reflection of the hook which crashed. For example “Crashed installing the software” for an install hook crash, or “Crash establishing database link” for a crash in a relationship hook.

Error state is not even part of the status-set principal states. This is cruft that can be removed and is extra errata that adds nothing to what status-set is actually doing.

And what is the purpose of the many examples when you can simply link to the reference section for juju states which contains that information? There's no example there for a programmer to consume in the varying languages that we have to implement charms, python, bash, powershell - as our targeted examples.

lazypower commented 8 years ago

I think the proper solution is finding a way to merge the two. To axe the wordy responses that are trying to be all encompassing, and expanding on the scant definitions that are output from the help-tool command, which will have real world benefits if we can issue a PR to juju core to update those output messages.

This does a few things

1) Reduces reader fatigue 2) Condenses the statements into useful, digestible information with actual code samples that are copy/pasteable for the stack overflow era of developer 3) Fixes some UX in the tooling itself 4) Ends a posturing debate about throwing away work that we aren't intending to just throw away.

Now how do we go from too much and too little to middle of the road where everyone is happy?

evilnick commented 8 years ago

I am not 'posturing', merely pointing out an important issue that I have already made at least once before and been ignored. I have never advocated we must choose one version over the other, but the title and opening comment of this issue does, which is why I responded to it.

to summarise, I AGREE a better version would:

lazypower commented 8 years ago

@evilnick - I've re-titled the bug to be less offensive

lazypower commented 8 years ago

AAAnnnddd has been merged.