On 02/21/2013 08:49 AM, Dan Callaghan wrote:
Lots of good ideas here, but to avoid this turning into a massive
monster of a mail I will split up my replies into separate mails.
Excerpts from Nick Coghlan's message of 2013-02-20 16:30:56 +1000:
> As a general point, these days we probably want to use
> application/json as the POST format rather than x-www-form-urlencoded
> form data.
I like form-encoding better because more tools support it directly -- in
particular, browsers. And I don't think there is anything in the API
where the request bodies have complex structures that would be better
represented in JSON. But we could easily make the API accept both
form-encoding and JSON objects.
Sounds good to me.
> As far as providing the recipe ID goes, perhaps a
BEAKER_RECIPE_ID
> environment var would be better than a file? That should be easier to
> make consistent cross-platform, and also matches the way we provide URL
> details.
Yes. Setting system-wide environment variables in a cross-platform way
has its own difficulties, as I mentioned, but it makes things simpler on
the harness side so it is probably the best choice. Although if we used
an environment variable for this I would just pick RECIPEID which is the
existing name that the harness exports to tasks.
The concern I have with a relatively generic name like "RECIPEID"
(depending on how generic you consider the term "RECIPE") is that we're
going to be encouraging the installation of other harnesses, and name
conflicts becomes a potential concern. I'd really like all of our
environment based configuration settings to have the "BEAKER_" prefix.
A task library based harness migrating to the stable API can easily do
"RECIPEID = BEAKER_RECIPEID" for compatibility with existing tasks.
> For the other two settings, how about BEAKER_API and BEAKER_HUB?
>
> The intent of BEAKER_API would be that API access is simply $BEAKER_API
> + documented API location (e.g. allowing us to expose different versions
> at different URLs if we decide we need to upgrade at some point in the
> future).
Yes you're right, we should really pass down the complete base URL for
the harness API. I wouldn't want people hardcoding port 8000 or a path
of / in their harness code, because we could never change it later.
I'm not sure BEAKER_API is a good name because Beaker has many APIs. And
I'm assuming here that any environment variables we set for the harness
will be set system-wide, just because that's the easiest thing to do. So
I would suggest any of the following, depending on what meaning we want
it to have:
- HARNESS_API
- BEAKER_HARNESS_API
- BEAKER_PROXY_BASE_URL
- BEAKER_LAB_CONTROLLER_BASE_URL
- LAB_CONTROLLER_BASE_URL
Now that I think about it, it makes more sense to expose the base URL of
the proxy (or of the lab controller, if you prefer to think of it that
way) and then define the harness API in terms of paths relative to that.
How about:
- BEAKER_LAB_URL
- BEAKER_HARNESS_API
I think there's an advantage to providing both:
1. providing BEAKER_LAB_URL means harnesses migrating from the old
XML-RPC API can work around any gaps in the stable API rather than it
being an "all-or-nothing" transition
2. providing BEAKER_HARNESS_API preserves *our* flexibility in the
overall URL design of the lab controller
> I'm not clear on why we need to provide BEAKER_HUB (don't
we want all
> access routed through the stable harness API?), but I prefer some kind
> of suffix over a bare BEAKER environment variable and HUB makes sense
> since we're going with --hub for the CLI option.
Yes in theory all access to the outside world should happen through the
proxy. But there is one big exception to that currently -- fetching
tasks from the task library.
And I agree that in hindsight calling the variable just BEAKER was
maybe
not wise, but this particular bike shed has already sailed :-) It's not
documented, because none of that stuff is, but that variable has been
available since 0.9.1 so I wouldn't want to take it away in case people
are relying on it.
As with RECIPEID above though, so long as people are aware of the
change, aliasing it back to the old name on the client side is fairly
trivial.
Given the rationale for providing it, I suggest BEAKER_TASK_LIBRARY_URL
as a new name. Harnesses that don't need to access the task library can
safely ignore it, those that expect it to be available under the old
name can alias it as "BEAKER = BEAKER_TASK_LIBRARY_URL"
Cheers,
Nick.
--
Nick Coghlan
Red Hat Infrastructure Engineering & Development, Brisbane
Python Applications Team Lead
Beaker Development Lead (
http://beaker-project.org/)
PulpDist Development Lead (
http://pulpdist.readthedocs.org)