Thanks all for the advice.
I think Geoff's email summarises the issue nicely. I like Alex's
suggestion but agree that limiting ourselves to deploy in the first is
probably significantly easier.
Personally I don't feel comfortable with using a tag for idempotency and I
do like Aled's suggestion of using PUT with a path with /{id} but would be
happy with either as I think they both fit our use case.
thanks
Duncan
On Wed, 26 Jul 2017 at 11:00 Geoff Macartney <[email protected]>
wrote:
> If I understand correctly this isn't quite what Duncan/Aled are asking for
> though?
> Which is not a "request id" but an idempotency token for an application. It
> would
> need to work long term, not just cached for a short time, and it would need
> to work
> across e.g. HA failover, so it wouldn't be just a matter of caching it on
> the server.
>
> For what it's worth, I'd have said this is a good use for tags but maybe
> for ease of reading
> it's better to have it as a config key as Aled does. As to how to supply
> the value
> I agree it should just be on the "deploy" operation.
>
> On Tue, 25 Jul 2017 at 19:56 Alex Heneveld <
> [email protected]>
> wrote:
>
> > Aled-
> >
> > Should this be applicable to all POST/DELETE calls? Imagine an
> > `X-caller-request-uid` and a filter which caches them server side for a
> > short period of time, blocking duplicates.
> >
> > Solves an overlapping set of problems. Your way deals with a
> > "deploy-if-not-present" much later in time.
> >
> > --A
> >
> > On 25 July 2017 at 17:44, Aled Sage <[email protected]> wrote:
> >
> > > Hi all,
> > >
> > > I've been exploring adding support for `&deploymentUid=...` - please
> see
> > > my work-in-progress PR [1].
> > >
> > > Do people think that is a better or worse direction than supporting
> > > `&appId=...` (which would likely be simpler code, but exposes the
> > Brooklyn
> > > internals more).
> > >
> > > For `&appId=...`, we could either revert [2] (so we could set the id in
> > > the EntitySpec), or we could inject it via a different (i.e. add a new)
> > > internal way so that it isn't exposedon our Java api classes.
> > >
> > > Thoughts?
> > >
> > > Aled
> > >
> > > [1] https://github.com/apache/brooklyn-server/pull/778
> > >
> > > [2] https://github.com/apache/brooklyn-server/pull/687/commits/5
> > > 5eb11fa91e9091447d56bb45116ccc3dc6009df
> > >
> > >
> > >
> > > On 07/07/2017 18:28, Aled Sage wrote:
> > >
> > >> Hi,
> > >>
> > >> Taking a step back to justify why this kind of thing is really
> > >> important...
> > >>
> > >> This has come up because we want to call Brooklyn in a robust way from
> > >> another system, and to handle a whole load of failure scenarios (e.g.
> > that
> > >> Brooklyn is temporarily down, connection fails at some point during
> the
> > >> communication, the client in the other system goes down and another
> > >> instance tries to pick up where it left off, etc).
> > >>
> > >> Those kind of thing becomes much easier if you can make certain
> > >> assumptions such as an API call being idempotent, or it guaranteeing
> to
> > >> fail with a given error if that exact request has already been
> accepted.
> > >>
> > >> ---
> > >>
> > >> I much prefer the semantics of the call failing (with a meaningful
> > error)
> > >> if the app already exists - that will make retry a lot easier to do
> > safely.
> > >>
> > >> As for config keys on the app, in Duncan's use-case he'd much prefer
> to
> > >> not mess with the user's YAML (e.g. to inject another config key
> before
> > >> passing it to Brooklyn). It would be simpler in his case to supply in
> > the
> > >> url `?appId=...` or `?deploymentId=...`.
> > >>
> > >> For using `deploymentId`, we could but that feels like more work. We'd
> > >> want create a lookup of applications indexed by `deploymentId` as well
> > as
> > >> `appId`, and to fail if it already exists. Also, what if someone also
> > >> defines a config key called `deploymentId` - would that be forbidden?
> Or
> > >> would we name-space the config key with
> > `org.apache.brooklyn.deploymentId`?
> > >> Even with those concerns, I could be persuaded of the
> > >> `org.apache.brooklyn.deploymentId` approach.
> > >>
> > >> For "/application's ID is not meant to be user-supplied/", that has
> > >> historically been the case but why should we stick to that? What
> > matters is
> > >> that the appId is definitely unique. That will be checked when
> creating
> > the
> > >> application entity. We could also include a regex check on the
> supplied
> > id
> > >> to make sure it looks reasonable (in case someone is already relying
> on
> > app
> > >> ids in weird ways like for filename generations, which would lead to a
> > risk
> > >> of script injection).
> > >>
> > >> Aled
> > >>
> > >>
> > >> On 07/07/2017 17:38, Svetoslav Neykov wrote:
> > >>
> > >>> Hi Duncan,
> > >>>
> > >>> I've solved this problem before by adding a caller generated config
> key
> > >>> on the app (now it's also possible to tag them), then iterating over
> > the
> > >>> deployed apps, looking for the key.
> > >>>
> > >>> An alternative which I'd like to mention is creating an async deploy
> > >>> operation which immediately returns an ID generated by Brooklyn.
> > There's
> > >>> still a window where the client connection could fail though, however
> > small
> > >>> it is, so it doesn't fully solve your use case.
> > >>>
> > >>> Your use case sounds reasonable so agree a solution to it would be
> nice
> > >>> to have.
> > >>>
> > >>> Svet.
> > >>>
> > >>>
> > >>> On 7.07.2017 г., at 18:33, Duncan Grant <
> > [email protected]>
> > >>>> wrote:
> > >>>>
> > >>>> I'd like to propose adding an appId parameter to the deploy
> endpoint.
> > >>>> This
> > >>>> would be optional and would presumably reject any attempt to start a
> > >>>> second
> > >>>> app with the same id. If set the appId would obviously be used in
> > >>>> place of
> > >>>> the generated id.
> > >>>>
> > >>>> This proposal would be of use in scripting deployments in a
> > distributed
> > >>>> environment where deployment is not the first step in a number of
> > >>>> asynchronous jobs and would give us a way of "connecting" those jobs
> > up.
> > >>>> Hopefully it will help a lot in making things more robust for
> > end-users.
> > >>>> Currently, if the client’s connection to the Brooklyn server fails
> > while
> > >>>> waiting for a response, it’s impossible to tell if the app was
> > >>>> provisioned
> > >>>> (e.g. how can you tell the difference between a likely-looking app,
> > and
> > >>>> another one deployed with an identical blueprint?). This would make
> it
> > >>>> safe
> > >>>> to either retry the deploy request, or to query for the app with the
> > >>>> expected id to see if it exists.
> > >>>>
> > >>>> Initially I'm hoping to use this in a downstream project but I think
> > >>>> this
> > >>>> would be useful to others.
> > >>>>
> > >>>> If no one has objections I'll aim to implement this over the next
> > >>>> couple of
> > >>>> weeks. On the other hand I'm totally open to suggestions of a
> better
> > >>>> approach.
> > >>>>
> > >>>> Thanks
> > >>>>
> > >>>> Duncan Grant
> > >>>>
> > >>>
> > >>
> > >>
> > >
> >
>
