Code review comment for lp:~cmars/juju-core/no-default-series

Revision history for this message
John A Meinel (jameinel) wrote :

It does mean 2 round trips for something that we are resolving in the
state server anyway, but it isn't a huge problem. Better than having
the client have to go to the Charm store directly. As you say, it
doesn't have to happen in this proposal, and this is probably still a
reasonable way forward.

On Mon, Mar 24, 2014 at 6:24 AM, Casey Marshall
<email address hidden> wrote:
> On 03/23/2014 04:30 AM, John A Meinel wrote:
>> On Thu, Mar 20, 2014 at 6:51 PM, Casey Marshall
>> <email address hidden> wrote:
>> ...
>>>
>>> When a client deploys or upgrades a charm, it is responsible for
>>> resolving the URL with the charm repository before making the API call.
>>> As far as I know, these are the only entry points for new charm URLs
>>> from the user into the API server. If this is naive, let's iterate on
>>> improving the defensiveness & I'm open to suggestion.
>>>
>>> I'd like the state server to never have to resolve a charm URL series,
>>> and reject all unresolved URLs.
>>
>> I'd like to understand this a bit better. Given the API server already
>> supports "deploy cs:precise/foo" which it will then go to the charm
>> store and download, it seems perfectly reasonable that the client
>> could connect to *1* remote machine (the API Server) rather than also
>> needing to know who the Charm Store is and how it is configured, and
>> how it can resolve information there.
>> Why do you feel it is better to push this work into needing Clients to
>> be smart? (One argument, I guess, is that Juju GUI is already listing
>> the charm store in order to present anything to clients, so the only
>> other client today is juju-core.)
>> But given that the API Server already needs to know how to talk to the
>> charm store (to install cs:precise/foo) it doesn't seem that bad to
>> have it also resolve URLs.
>>
>
> My primary goal there is to keep the state server simple, backwards-
> (and forwards-) compatible. I decided to keep the current requirements
> of a fully-resolved charm URL in deploy, publish, and
> upgradecharm-related API calls. In my latest changes to this proposal,
> this is actually enforced by the type-system -- there is a distinction
> between charm.URL (a fully-resolved charm location with series) and
> charm.Reference (a charm location without a series resolved).
>
> You raise a good point that the (>= 1.18) juju client will not contact
> the charm store directly. What would you think of adding an API method,
> call it "ResolveCharm", which exposes the charm store series-resolving
> functionality back to the state client? The state client would still be
> responsible for resolving the charm, but it will not need to contact the
> charm store directly to resolve it.
>
> I don't think "ResolveCharm" would necessarily need to land in this
> merge proposal. It could go into the follow-up proposal with the
> client-side changes.
>
>> John
>> =:->
>>
>
> -Casey
>
>
> --
> https://code.launchpad.net/~cmars/juju-core/no-default-series/+merge/211389
> You are subscribed to branch lp:juju-core.

« Back to merge proposal