Proposal: Add appId optional paramater to deploy api

[Duncan Grant <du...@cloudsoftcorp.com>]

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

Re: Proposal: Add appId optional paramater to deploy api

[Aled Sage <al...@gmail.com>]

// apologies this ended up so long!

Hi Alex, Svet, all,

Interesting suggestion by Alex - sounds useful. However, even with that 
it would require implementing a big chunk of logic in the upstream system.

I keep coming back to thinking that `appId=...` gives the simplest 
semantics for such systems.

---
The caching of `X-Request-ID` approach doesn't work in the scenario when 
the Brooklyn process has restarted, or has failed over in HA mode (I'm 
presuming you don't intend to write them to persisted state).

We'd therefore still end up with an upstream implementation as though 
`X-Request-ID` wasn't supported.

We'd still want either `appId=...` or `deploymentUid=...` so that we can 
subsequently query to find if there is a given orphaned application.

It would be nice if the query was simple/efficient. That's easy if using 
appId, but fiddly to search for an app with a given `deploymentUid` 
config value or tag (I think currently it would require listing all 
apps, then querying each app for this config key; or changing brooklyn 
to expand the rest api; or running an additional custom web-app for such 
bespoke queries).

---
I think it's fine/sensible to focus on the specific problem of 
deploy-app, because that is the current use-case.

Separately, having a general solution that works in some situations is a 
nice addition, but feels like a lower priority for our use-case.

---
I presume if implementing the `X-Request-ID` cache:

  * The cache would be transient (i.e. not persisted for restart/HA).
  * It would record everything with side-effects (POST/PUT/DELETE, but
    not GET/HEAD).
  * We'd record it as soon as the request was received (rather than
    waiting for the request to complete).
  * We'd return a 409 (conflict) response if we received a duplicate.
  * It would be configurable to turn this feature on/off (defaulting to on).
  * The time for keeping these request-ids would be configurable,
    defaulting to something like an hour.

Assuming folk agree it's separate, then let's treat that as a separate 
proposal / email thread.

---

Mark has pointed out that strictly speaking we shouldn't be using query 
parameters with POST requests.

The RESTful way of doing it if supplying a custom app id would be:

     `PUT /applications/<custom-app-id>`

It's not so obvious how best to supply a deploymentUid in the url.

---

To Svet's point of using a tag (if going for the `deploymentUid` 
approach), good point - agree that would probably be better. We'd 
probably add it as the string `org.apache.brooklyn.deploymentUid=<val>`.

It would have been nicer (in this use-case and various others) if tags 
was a map rather than a list, but we could live with adding the uid to 
the list.

---

p.s. To again summarise the requirements/thinking: we want to make it 
easier for upstream systems to use Brooklyn robustly. We want to more 
easily/safely handle retries, and to be able to easily query for and/or 
discover orphaned apps (where a request got through, but response was 
lost). Our primary use-case in the upsteram system is deploying apps. We 
don't want to modify the user's yaml files.

Aled



On 26/07/2017 10:16, Svetoslav Neykov wrote:
> I think both proposals make sense. Even if implemented separately.
>
> Wondering what's the right place to put the ID though. Isn't the tags a better place for that?  I suppose it depends on whether we want YAML blueprints to have access to it.
>
> Svet.
>
>
>> On 25.07.2017 г., at 21:56, Alex Heneveld <al...@cloudsoftcorp.com> 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 <al...@gmail.com> 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 <du...@cloudsoftcorp.com>
>>>>>> 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
>>>>>>
>>>>

Re: Proposal: Add appId optional paramater to deploy api

[Svetoslav Neykov <sv...@cloudsoft.io>]

I think both proposals make sense. Even if implemented separately.

Wondering what's the right place to put the ID though. Isn't the tags a better place for that?  I suppose it depends on whether we want YAML blueprints to have access to it.

Svet. 


> On 25.07.2017 г., at 21:56, Alex Heneveld <al...@cloudsoftcorp.com> 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 <al...@gmail.com> 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 <du...@cloudsoftcorp.com>
>>>>> 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
>>>>> 
>>>> 
>>> 
>>> 
>> 

Re: Proposal: Add appId optional paramater to deploy api

[Aled Sage <al...@gmail.com>]

Thanks Alex,

Yes - we can say it's `Beta` in the rest api description, and in the 
plumming code for however we pass the app-id through.

I'll delete my existing PR, and submit a new one based on this approach.

Aled


On 27/07/2017 08:12, Alex Heneveld wrote:
>
> Thanks Aled -- the inheritance of config from catalog items convinces me.
>
> Can we mark it @Beta / internal in case we need to change the 
> approach?  With that I'd be happy with your proposal.
>
> Best
> Alex
>
>
> On 27/07/2017 07:23, Aled Sage wrote:
>> Hi Alex,
>>
>> I explored setting a config key in my PR. The downsides of that 
>> compared to setting the app-id:
>>
>> 1. Code is more complicated - in particular, there are very low-level 
>> changes to check that the uid is not already in use.
>> 2. Config keys (and tags) are mutable - we can't enforce the proposed 
>> semantics of the deploymentUid.
>> 3. You can always rely on looking up an entity's id, but config is 
>> more complicated and can be "resolved" - leads to more complicated 
>> code (e.g. my PR currently has test-failures for hot-standby).
>> 4. Appropriate search not supported in the REST api (would require 
>> additional changes - i.e. more work).
>> 5. Server-side searching would either be inefficient or require a new 
>> index to be exposed (e.g. low-level changes in `EntityManager`).
>> 6. Adds another concept - a new kind of id.
>>
>> ---
>> By "camp.id", do you mean "camp.plan.id"? (there is also 
>> "camp.template.id", but I can't find a "camp.id".)
>>
>> I don't think we should mix this up with that existing concept, which 
>> is set and used in a completely different way:
>> * One can already set a camp id on the top-level app, and reference 
>> it with `$brooklyn:entity(...)` - we don't want to break that.
>> * The camp id does not have to be unique - changing that would break 
>> backwards compatibility.
>> * Existing catalog items can set the camp id, which are used by the 
>> app instance - e.g. see example in the appendix.
>>
>> ---
>> What are your main concerns about allowing the id to be set (with the 
>> regex validation that Mark suggested)? The reason that the is used by 
>> other internal parts of Brooklyn seems too vague to me. For example, 
>> is it:
>> 1. Security (e.g. if we didn't validate, then it would risk allowing 
>> code injection).
>> 2. Makes future changes harder (but the sorts of changes I envisage I 
>> can also see how we could handle).
>> 3. Point of principle to keep internal things internal wherever 
>> possible.
>> 4. Risks breaking places that we use the id in strange ways (e.g. if 
>> an entity uses the id to generate a dns name, then it implies 
>> case-insensitivity for the uniqueness check).
>>
>> That last concern is real - I recall that Andrew Kennedy changed our 
>> ids to be lower case for that reason!
>>
>> However, I don't think we should have given such "undocumented 
>> guarantees" on the internal id format. But given some entities rely 
>> on it, we can be stricter with the validation regex of user-supplied 
>> ids.
>>
>> Aled
>>
>> ======= Appendix =======
>> Example of setting a `camp.plan.id` - we don't want to subtly change 
>> the semantics of this.
>>
>> Add this to the catalog:
>> ```
>> brooklyn.catalog:
>>   id: app-with-camp-plan-id
>>   version: 0.0.0-SNAPSHOT
>>   itemType: template
>>   item:
>>     services:
>>       - type: org.apache.brooklyn.entity.stock.BasicApplication
>>         id: myAppCampPlanId
>> ```
>>
>> Deploy this app:
>> ```
>> services:
>>   - type: app-with-camp-plan-id
>> ```
>>
>> The resulting app instance will have config `camp.plan.id` with value 
>> `myAppCampPlanId`.
>>
>>
>>
>> On 27/07/2017 00:40, Alex Heneveld wrote:
>>> The core `id` is a low-level part of `BrooklynObject` used by all 
>>> adjuncts
>>> and entities and persistence.  It feels wrong and risky making this
>>> something that is user- or client- settable.  I gave one example but 
>>> there
>>> are others.
>>>
>>> What's wrong with a new config key or reusing `camp.id`?  We already 
>>> use
>>> the latter one if there is a user-specified ID on an entity so it feels
>>> natural to use it, give it special meaning for apps (blocking repeat
>>> deployments), and add support for searching for it. (Apologies if 
>>> this was
>>> explained and I missed it.)
>>>
>>> --A
>>>
>>>
>>> On 26 July 2017 at 22:42, Aled Sage <al...@gmail.com> wrote:
>>>
>>>> Hi Alex,
>>>>
>>>> Other things get a lot simpler for us if we can just supply the app-id
>>>> (e.g. subsequently searching for the app, or ensuring that a 
>>>> duplicate app
>>>> is not deployed). It also means we're not adding another concept 
>>>> that we
>>>> need to explain to users.
>>>>
>>>> To me, that simplicity is very compelling.
>>>>
>>>> If we want to support conformance to external id requirements, we 
>>>> could
>>>> have a config key for a predicate or regex that the supplied id must
>>>> satisfy. A given user could thus enforce their id standards in the 
>>>> future.
>>>> I'd favour deferring that until there is a need to support it (e.g. we
>>>> could add it at the same time as adding support for a pluggable id
>>>> generator, if we ever do that).
>>>>
>>>> Aled
>>>>
>>>>
>>>>
>>>> On 26/07/2017 15:42, Alex Heneveld wrote:
>>>>
>>>>> 2 feels compelling to me. I want us to have the ability easily to 
>>>>> change
>>>>> the ID generation eg to conform with external reqs such as 
>>>>> timestamp or
>>>>> source.
>>>>>
>>>>> Go with deploymentUid or similar? Or camp.id?
>>>>>
>>>>> Best
>>>>> Alex
>>>>>
>>>>> On 26 Jul 2017 15:00, "Aled Sage" <al...@gmail.com> wrote:
>>>>>
>>>>> Hi Mark,
>>>>>
>>>>> We removed from EntitySpec the ability to set the id for two reasons:
>>>>>
>>>>> 1. there was no use-case at that time; simplifying the code by 
>>>>> deleting it
>>>>> was therefore sensible - we'd deprecated it for several releases.
>>>>>
>>>>> 2. allowing all uids to be generated/managed internally is simpler to
>>>>> reason about, and gives greatest freedom for future refactoring.
>>>>>
>>>>>
>>>>> I don't see (2) as a compelling reason.  And we now have a 
>>>>> use-case, so
>>>>> that changes (1).
>>>>>
>>>>> I'd still be tempted to treat this as an internal part of the api, 
>>>>> rather
>>>>> than it going on the public `EntitySpec`, but need to look at that 
>>>>> more to
>>>>> see how feasible it is.
>>>>>
>>>>> Aled
>>>>>
>>>>>
>>>>>
>>>>> On 26/07/2017 13:36, Mark Mc Kenna wrote:
>>>>>
>>>>> Thanks Geoff for the good summary
>>>>>> IMO the path of least resistance that provides the best / most
>>>>>> predictable
>>>>>> behaviour is allowing clients to optionally set the app id.
>>>>>>
>>>>>> Off the top of my head I cant see this causing any issue, as long 
>>>>>> as we
>>>>>> sanitise the supplied id something like [a-zA-Z0-9-]{8,}.
>>>>>>
>>>>>> Was there a particular reason this was removed in the past?
>>>>>>
>>>>>> Cheers
>>>>>> M
>>>>>>
>>>>>> On 26 July 2017 at 13:07, Duncan Grant 
>>>>>> <du...@cloudsoftcorp.com>
>>>>>> wrote:
>>>>>>
>>>>>> 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 <
>>>>>>> geoff.macartney@cloudsoft.io
>>>>>>> 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 <
>>>>>>>> alex.heneveld@cloudsoftcorp.com>
>>>>>>>> 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 <al...@gmail.com> 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 <
>>>>>>>>>>>>
>>>>>>>>>>>> duncan.grant@cloudsoftcorp.com>
>>>>>>>>>> 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
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>
>

Re: Proposal: Add appId optional paramater to deploy api

[Alex Heneveld <al...@cloudsoftcorp.com>]

Thanks Aled -- the inheritance of config from catalog items convinces me.

Can we mark it @Beta / internal in case we need to change the approach?  
With that I'd be happy with your proposal.

Best
Alex


On 27/07/2017 07:23, Aled Sage wrote:
> Hi Alex,
>
> I explored setting a config key in my PR. The downsides of that 
> compared to setting the app-id:
>
> 1. Code is more complicated - in particular, there are very low-level 
> changes to check that the uid is not already in use.
> 2. Config keys (and tags) are mutable - we can't enforce the proposed 
> semantics of the deploymentUid.
> 3. You can always rely on looking up an entity's id, but config is 
> more complicated and can be "resolved" - leads to more complicated 
> code (e.g. my PR currently has test-failures for hot-standby).
> 4. Appropriate search not supported in the REST api (would require 
> additional changes - i.e. more work).
> 5. Server-side searching would either be inefficient or require a new 
> index to be exposed (e.g. low-level changes in `EntityManager`).
> 6. Adds another concept - a new kind of id.
>
> ---
> By "camp.id", do you mean "camp.plan.id"? (there is also 
> "camp.template.id", but I can't find a "camp.id".)
>
> I don't think we should mix this up with that existing concept, which 
> is set and used in a completely different way:
> * One can already set a camp id on the top-level app, and reference it 
> with `$brooklyn:entity(...)` - we don't want to break that.
> * The camp id does not have to be unique - changing that would break 
> backwards compatibility.
> * Existing catalog items can set the camp id, which are used by the 
> app instance - e.g. see example in the appendix.
>
> ---
> What are your main concerns about allowing the id to be set (with the 
> regex validation that Mark suggested)? The reason that the is used by 
> other internal parts of Brooklyn seems too vague to me. For example, 
> is it:
> 1. Security (e.g. if we didn't validate, then it would risk allowing 
> code injection).
> 2. Makes future changes harder (but the sorts of changes I envisage I 
> can also see how we could handle).
> 3. Point of principle to keep internal things internal wherever possible.
> 4. Risks breaking places that we use the id in strange ways (e.g. if 
> an entity uses the id to generate a dns name, then it implies 
> case-insensitivity for the uniqueness check).
>
> That last concern is real - I recall that Andrew Kennedy changed our 
> ids to be lower case for that reason!
>
> However, I don't think we should have given such "undocumented 
> guarantees" on the internal id format. But given some entities rely on 
> it, we can be stricter with the validation regex of user-supplied ids.
>
> Aled
>
> ======= Appendix =======
> Example of setting a `camp.plan.id` - we don't want to subtly change 
> the semantics of this.
>
> Add this to the catalog:
> ```
> brooklyn.catalog:
>   id: app-with-camp-plan-id
>   version: 0.0.0-SNAPSHOT
>   itemType: template
>   item:
>     services:
>       - type: org.apache.brooklyn.entity.stock.BasicApplication
>         id: myAppCampPlanId
> ```
>
> Deploy this app:
> ```
> services:
>   - type: app-with-camp-plan-id
> ```
>
> The resulting app instance will have config `camp.plan.id` with value 
> `myAppCampPlanId`.
>
>
>
> On 27/07/2017 00:40, Alex Heneveld wrote:
>> The core `id` is a low-level part of `BrooklynObject` used by all 
>> adjuncts
>> and entities and persistence.  It feels wrong and risky making this
>> something that is user- or client- settable.  I gave one example but 
>> there
>> are others.
>>
>> What's wrong with a new config key or reusing `camp.id`?  We already use
>> the latter one if there is a user-specified ID on an entity so it feels
>> natural to use it, give it special meaning for apps (blocking repeat
>> deployments), and add support for searching for it.  (Apologies if 
>> this was
>> explained and I missed it.)
>>
>> --A
>>
>>
>> On 26 July 2017 at 22:42, Aled Sage <al...@gmail.com> wrote:
>>
>>> Hi Alex,
>>>
>>> Other things get a lot simpler for us if we can just supply the app-id
>>> (e.g. subsequently searching for the app, or ensuring that a 
>>> duplicate app
>>> is not deployed). It also means we're not adding another concept 
>>> that we
>>> need to explain to users.
>>>
>>> To me, that simplicity is very compelling.
>>>
>>> If we want to support conformance to external id requirements, we could
>>> have a config key for a predicate or regex that the supplied id must
>>> satisfy. A given user could thus enforce their id standards in the 
>>> future.
>>> I'd favour deferring that until there is a need to support it (e.g. we
>>> could add it at the same time as adding support for a pluggable id
>>> generator, if we ever do that).
>>>
>>> Aled
>>>
>>>
>>>
>>> On 26/07/2017 15:42, Alex Heneveld wrote:
>>>
>>>> 2 feels compelling to me. I want us to have the ability easily to 
>>>> change
>>>> the ID generation eg to conform with external reqs such as 
>>>> timestamp or
>>>> source.
>>>>
>>>> Go with deploymentUid or similar? Or camp.id?
>>>>
>>>> Best
>>>> Alex
>>>>
>>>> On 26 Jul 2017 15:00, "Aled Sage" <al...@gmail.com> wrote:
>>>>
>>>> Hi Mark,
>>>>
>>>> We removed from EntitySpec the ability to set the id for two reasons:
>>>>
>>>> 1. there was no use-case at that time; simplifying the code by 
>>>> deleting it
>>>> was therefore sensible - we'd deprecated it for several releases.
>>>>
>>>> 2. allowing all uids to be generated/managed internally is simpler to
>>>> reason about, and gives greatest freedom for future refactoring.
>>>>
>>>>
>>>> I don't see (2) as a compelling reason.  And we now have a 
>>>> use-case, so
>>>> that changes (1).
>>>>
>>>> I'd still be tempted to treat this as an internal part of the api, 
>>>> rather
>>>> than it going on the public `EntitySpec`, but need to look at that 
>>>> more to
>>>> see how feasible it is.
>>>>
>>>> Aled
>>>>
>>>>
>>>>
>>>> On 26/07/2017 13:36, Mark Mc Kenna wrote:
>>>>
>>>> Thanks Geoff for the good summary
>>>>> IMO the path of least resistance that provides the best / most
>>>>> predictable
>>>>> behaviour is allowing clients to optionally set the app id.
>>>>>
>>>>> Off the top of my head I cant see this causing any issue, as long 
>>>>> as we
>>>>> sanitise the supplied id something like [a-zA-Z0-9-]{8,}.
>>>>>
>>>>> Was there a particular reason this was removed in the past?
>>>>>
>>>>> Cheers
>>>>> M
>>>>>
>>>>> On 26 July 2017 at 13:07, Duncan Grant 
>>>>> <du...@cloudsoftcorp.com>
>>>>> wrote:
>>>>>
>>>>> 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 <
>>>>>> geoff.macartney@cloudsoft.io
>>>>>> 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 <
>>>>>>> alex.heneveld@cloudsoftcorp.com>
>>>>>>> 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 <al...@gmail.com> 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 <
>>>>>>>>>>>
>>>>>>>>>>> duncan.grant@cloudsoftcorp.com>
>>>>>>>>> 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
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>

Re: Proposal: Add appId optional paramater to deploy api

[Aled Sage <al...@gmail.com>]

Hi Alex,

I explored setting a config key in my PR. The downsides of that compared 
to setting the app-id:

1. Code is more complicated - in particular, there are very low-level 
changes to check that the uid is not already in use.
2. Config keys (and tags) are mutable - we can't enforce the proposed 
semantics of the deploymentUid.
3. You can always rely on looking up an entity's id, but config is more 
complicated and can be "resolved" - leads to more complicated code (e.g. 
my PR currently has test-failures for hot-standby).
4. Appropriate search not supported in the REST api (would require 
additional changes - i.e. more work).
5. Server-side searching would either be inefficient or require a new 
index to be exposed (e.g. low-level changes in `EntityManager`).
6. Adds another concept - a new kind of id.

---
By "camp.id", do you mean "camp.plan.id"? (there is also 
"camp.template.id", but I can't find a "camp.id".)

I don't think we should mix this up with that existing concept, which is 
set and used in a completely different way:
* One can already set a camp id on the top-level app, and reference it 
with `$brooklyn:entity(...)` - we don't want to break that.
* The camp id does not have to be unique - changing that would break 
backwards compatibility.
* Existing catalog items can set the camp id, which are used by the app 
instance - e.g. see example in the appendix.

---
What are your main concerns about allowing the id to be set (with the 
regex validation that Mark suggested)? The reason that the is used by 
other internal parts of Brooklyn seems too vague to me. For example, is it:
1. Security (e.g. if we didn't validate, then it would risk allowing 
code injection).
2. Makes future changes harder (but the sorts of changes I envisage I 
can also see how we could handle).
3. Point of principle to keep internal things internal wherever possible.
4. Risks breaking places that we use the id in strange ways (e.g. if an 
entity uses the id to generate a dns name, then it implies 
case-insensitivity for the uniqueness check).

That last concern is real - I recall that Andrew Kennedy changed our ids 
to be lower case for that reason!

However, I don't think we should have given such "undocumented 
guarantees" on the internal id format. But given some entities rely on 
it, we can be stricter with the validation regex of user-supplied ids.

Aled

======= Appendix =======
Example of setting a `camp.plan.id` - we don't want to subtly change the 
semantics of this.

Add this to the catalog:
```
brooklyn.catalog:
   id: app-with-camp-plan-id
   version: 0.0.0-SNAPSHOT
   itemType: template
   item:
     services:
       - type: org.apache.brooklyn.entity.stock.BasicApplication
         id: myAppCampPlanId
```

Deploy this app:
```
services:
   - type: app-with-camp-plan-id
```

The resulting app instance will have config `camp.plan.id` with value 
`myAppCampPlanId`.



On 27/07/2017 00:40, Alex Heneveld wrote:
> The core `id` is a low-level part of `BrooklynObject` used by all adjuncts
> and entities and persistence.  It feels wrong and risky making this
> something that is user- or client- settable.  I gave one example but there
> are others.
>
> What's wrong with a new config key or reusing `camp.id`?  We already use
> the latter one if there is a user-specified ID on an entity so it feels
> natural to use it, give it special meaning for apps (blocking repeat
> deployments), and add support for searching for it.  (Apologies if this was
> explained and I missed it.)
>
> --A
>
>
> On 26 July 2017 at 22:42, Aled Sage <al...@gmail.com> wrote:
>
>> Hi Alex,
>>
>> Other things get a lot simpler for us if we can just supply the app-id
>> (e.g. subsequently searching for the app, or ensuring that a duplicate app
>> is not deployed). It also means we're not adding another concept that we
>> need to explain to users.
>>
>> To me, that simplicity is very compelling.
>>
>> If we want to support conformance to external id requirements, we could
>> have a config key for a predicate or regex that the supplied id must
>> satisfy. A given user could thus enforce their id standards in the future.
>> I'd favour deferring that until there is a need to support it (e.g. we
>> could add it at the same time as adding support for a pluggable id
>> generator, if we ever do that).
>>
>> Aled
>>
>>
>>
>> On 26/07/2017 15:42, Alex Heneveld wrote:
>>
>>> 2 feels compelling to me. I want us to have the ability easily to change
>>> the ID generation eg to conform with external reqs such as timestamp or
>>> source.
>>>
>>> Go with deploymentUid or similar? Or camp.id?
>>>
>>> Best
>>> Alex
>>>
>>> On 26 Jul 2017 15:00, "Aled Sage" <al...@gmail.com> wrote:
>>>
>>> Hi Mark,
>>>
>>> We removed from EntitySpec the ability to set the id for two reasons:
>>>
>>> 1. there was no use-case at that time; simplifying the code by deleting it
>>> was therefore sensible - we'd deprecated it for several releases.
>>>
>>> 2. allowing all uids to be generated/managed internally is simpler to
>>> reason about, and gives greatest freedom for future refactoring.
>>>
>>>
>>> I don't see (2) as a compelling reason.  And we now have a use-case, so
>>> that changes (1).
>>>
>>> I'd still be tempted to treat this as an internal part of the api, rather
>>> than it going on the public `EntitySpec`, but need to look at that more to
>>> see how feasible it is.
>>>
>>> Aled
>>>
>>>
>>>
>>> On 26/07/2017 13:36, Mark Mc Kenna wrote:
>>>
>>> Thanks Geoff for the good summary
>>>> IMO the path of least resistance that provides the best / most
>>>> predictable
>>>> behaviour is allowing clients to optionally set the app id.
>>>>
>>>> Off the top of my head I cant see this causing any issue, as long as we
>>>> sanitise the supplied id something like [a-zA-Z0-9-]{8,}.
>>>>
>>>> Was there a particular reason this was removed in the past?
>>>>
>>>> Cheers
>>>> M
>>>>
>>>> On 26 July 2017 at 13:07, Duncan Grant <du...@cloudsoftcorp.com>
>>>> wrote:
>>>>
>>>> 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 <
>>>>> geoff.macartney@cloudsoft.io
>>>>> 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 <
>>>>>> alex.heneveld@cloudsoftcorp.com>
>>>>>> 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 <al...@gmail.com> 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 <
>>>>>>>>>>
>>>>>>>>>> duncan.grant@cloudsoftcorp.com>
>>>>>>>> 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
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>

Re: Proposal: Add appId optional paramater to deploy api

[Alex Heneveld <al...@cloudsoftcorp.com>]

The core `id` is a low-level part of `BrooklynObject` used by all adjuncts
and entities and persistence.  It feels wrong and risky making this
something that is user- or client- settable.  I gave one example but there
are others.

What's wrong with a new config key or reusing `camp.id`?  We already use
the latter one if there is a user-specified ID on an entity so it feels
natural to use it, give it special meaning for apps (blocking repeat
deployments), and add support for searching for it.  (Apologies if this was
explained and I missed it.)

--A


On 26 July 2017 at 22:42, Aled Sage <al...@gmail.com> wrote:

> Hi Alex,
>
> Other things get a lot simpler for us if we can just supply the app-id
> (e.g. subsequently searching for the app, or ensuring that a duplicate app
> is not deployed). It also means we're not adding another concept that we
> need to explain to users.
>
> To me, that simplicity is very compelling.
>
> If we want to support conformance to external id requirements, we could
> have a config key for a predicate or regex that the supplied id must
> satisfy. A given user could thus enforce their id standards in the future.
> I'd favour deferring that until there is a need to support it (e.g. we
> could add it at the same time as adding support for a pluggable id
> generator, if we ever do that).
>
> Aled
>
>
>
> On 26/07/2017 15:42, Alex Heneveld wrote:
>
>> 2 feels compelling to me. I want us to have the ability easily to change
>> the ID generation eg to conform with external reqs such as timestamp or
>> source.
>>
>> Go with deploymentUid or similar? Or camp.id?
>>
>> Best
>> Alex
>>
>> On 26 Jul 2017 15:00, "Aled Sage" <al...@gmail.com> wrote:
>>
>> Hi Mark,
>>
>> We removed from EntitySpec the ability to set the id for two reasons:
>>
>> 1. there was no use-case at that time; simplifying the code by deleting it
>> was therefore sensible - we'd deprecated it for several releases.
>>
>> 2. allowing all uids to be generated/managed internally is simpler to
>> reason about, and gives greatest freedom for future refactoring.
>>
>>
>> I don't see (2) as a compelling reason.  And we now have a use-case, so
>> that changes (1).
>>
>> I'd still be tempted to treat this as an internal part of the api, rather
>> than it going on the public `EntitySpec`, but need to look at that more to
>> see how feasible it is.
>>
>> Aled
>>
>>
>>
>> On 26/07/2017 13:36, Mark Mc Kenna wrote:
>>
>> Thanks Geoff for the good summary
>>>
>>> IMO the path of least resistance that provides the best / most
>>> predictable
>>> behaviour is allowing clients to optionally set the app id.
>>>
>>> Off the top of my head I cant see this causing any issue, as long as we
>>> sanitise the supplied id something like [a-zA-Z0-9-]{8,}.
>>>
>>> Was there a particular reason this was removed in the past?
>>>
>>> Cheers
>>> M
>>>
>>> On 26 July 2017 at 13:07, Duncan Grant <du...@cloudsoftcorp.com>
>>> wrote:
>>>
>>> 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 <
>>>> geoff.macartney@cloudsoft.io
>>>> 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 <
>>>>> alex.heneveld@cloudsoftcorp.com>
>>>>> 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 <al...@gmail.com> 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 <
>>>>>>>>>
>>>>>>>>> duncan.grant@cloudsoftcorp.com>
>>>>>>>>
>>>>>>> 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
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>

Re: Proposal: Add appId optional paramater to deploy api

[Aled Sage <al...@gmail.com>]

Hi Alex,

Other things get a lot simpler for us if we can just supply the app-id 
(e.g. subsequently searching for the app, or ensuring that a duplicate 
app is not deployed). It also means we're not adding another concept 
that we need to explain to users.

To me, that simplicity is very compelling.

If we want to support conformance to external id requirements, we could 
have a config key for a predicate or regex that the supplied id must 
satisfy. A given user could thus enforce their id standards in the 
future. I'd favour deferring that until there is a need to support it 
(e.g. we could add it at the same time as adding support for a pluggable 
id generator, if we ever do that).

Aled


On 26/07/2017 15:42, Alex Heneveld wrote:
> 2 feels compelling to me. I want us to have the ability easily to change
> the ID generation eg to conform with external reqs such as timestamp or
> source.
>
> Go with deploymentUid or similar? Or camp.id?
>
> Best
> Alex
>
> On 26 Jul 2017 15:00, "Aled Sage" <al...@gmail.com> wrote:
>
> Hi Mark,
>
> We removed from EntitySpec the ability to set the id for two reasons:
>
> 1. there was no use-case at that time; simplifying the code by deleting it
> was therefore sensible - we'd deprecated it for several releases.
>
> 2. allowing all uids to be generated/managed internally is simpler to
> reason about, and gives greatest freedom for future refactoring.
>
>
> I don't see (2) as a compelling reason.  And we now have a use-case, so
> that changes (1).
>
> I'd still be tempted to treat this as an internal part of the api, rather
> than it going on the public `EntitySpec`, but need to look at that more to
> see how feasible it is.
>
> Aled
>
>
>
> On 26/07/2017 13:36, Mark Mc Kenna wrote:
>
>> Thanks Geoff for the good summary
>>
>> IMO the path of least resistance that provides the best / most predictable
>> behaviour is allowing clients to optionally set the app id.
>>
>> Off the top of my head I cant see this causing any issue, as long as we
>> sanitise the supplied id something like [a-zA-Z0-9-]{8,}.
>>
>> Was there a particular reason this was removed in the past?
>>
>> Cheers
>> M
>>
>> On 26 July 2017 at 13:07, Duncan Grant <du...@cloudsoftcorp.com>
>> wrote:
>>
>> 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 <
>>> geoff.macartney@cloudsoft.io
>>> 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 <
>>>> alex.heneveld@cloudsoftcorp.com>
>>>> 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 <al...@gmail.com> 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 <
>>>>>>>>
>>>>>>> duncan.grant@cloudsoftcorp.com>
>>>>>> 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
>>>>>>>>>
>>>>>>>>>

Re: Proposal: Add appId optional paramater to deploy api

[Alex Heneveld <al...@cloudsoftcorp.com>]

2 feels compelling to me. I want us to have the ability easily to change
the ID generation eg to conform with external reqs such as timestamp or
source.

Go with deploymentUid or similar? Or camp.id?

Best
Alex

On 26 Jul 2017 15:00, "Aled Sage" <al...@gmail.com> wrote:

Hi Mark,

We removed from EntitySpec the ability to set the id for two reasons:

1. there was no use-case at that time; simplifying the code by deleting it
was therefore sensible - we'd deprecated it for several releases.

2. allowing all uids to be generated/managed internally is simpler to
reason about, and gives greatest freedom for future refactoring.


I don't see (2) as a compelling reason.  And we now have a use-case, so
that changes (1).

I'd still be tempted to treat this as an internal part of the api, rather
than it going on the public `EntitySpec`, but need to look at that more to
see how feasible it is.

Aled



On 26/07/2017 13:36, Mark Mc Kenna wrote:

> Thanks Geoff for the good summary
>
> IMO the path of least resistance that provides the best / most predictable
> behaviour is allowing clients to optionally set the app id.
>
> Off the top of my head I cant see this causing any issue, as long as we
> sanitise the supplied id something like [a-zA-Z0-9-]{8,}.
>
> Was there a particular reason this was removed in the past?
>
> Cheers
> M
>
> On 26 July 2017 at 13:07, Duncan Grant <du...@cloudsoftcorp.com>
> wrote:
>
> 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 <
>> geoff.macartney@cloudsoft.io
>> 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 <
>>> alex.heneveld@cloudsoftcorp.com>
>>> 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 <al...@gmail.com> 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 <
>>>>>>>
>>>>>> duncan.grant@cloudsoftcorp.com>
>>>>
>>>>> 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
>>>>>>>>
>>>>>>>>
>>>>>>

Re: Proposal: Add appId optional paramater to deploy api

[Aled Sage <al...@gmail.com>]

Hi Mark,

We removed from EntitySpec the ability to set the id for two reasons:

1. there was no use-case at that time; simplifying the code by deleting 
it was therefore sensible - we'd deprecated it for several releases.

2. allowing all uids to be generated/managed internally is simpler to 
reason about, and gives greatest freedom for future refactoring.


I don't see (2) as a compelling reason.  And we now have a use-case, so 
that changes (1).

I'd still be tempted to treat this as an internal part of the api, 
rather than it going on the public `EntitySpec`, but need to look at 
that more to see how feasible it is.

Aled


On 26/07/2017 13:36, Mark Mc Kenna wrote:
> Thanks Geoff for the good summary
>
> IMO the path of least resistance that provides the best / most predictable
> behaviour is allowing clients to optionally set the app id.
>
> Off the top of my head I cant see this causing any issue, as long as we
> sanitise the supplied id something like [a-zA-Z0-9-]{8,}.
>
> Was there a particular reason this was removed in the past?
>
> Cheers
> M
>
> On 26 July 2017 at 13:07, Duncan Grant <du...@cloudsoftcorp.com>
> wrote:
>
>> 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 <geoff.macartney@cloudsoft.io
>> 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 <
>>> alex.heneveld@cloudsoftcorp.com>
>>> 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 <al...@gmail.com> 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 <
>>>> duncan.grant@cloudsoftcorp.com>
>>>>>>>> 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
>>>>>>>>
>>>>>>

Re: Proposal: Add appId optional paramater to deploy api

[Mark Mc Kenna <m4...@apache.org>]

Thanks Geoff for the good summary

IMO the path of least resistance that provides the best / most predictable
behaviour is allowing clients to optionally set the app id.

Off the top of my head I cant see this causing any issue, as long as we
sanitise the supplied id something like [a-zA-Z0-9-]{8,}.

Was there a particular reason this was removed in the past?

Cheers
M

On 26 July 2017 at 13:07, Duncan Grant <du...@cloudsoftcorp.com>
wrote:

> 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 <geoff.macartney@cloudsoft.io
> >
> 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 <
> > alex.heneveld@cloudsoftcorp.com>
> > 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 <al...@gmail.com> 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 <
> > > duncan.grant@cloudsoftcorp.com>
> > > >>>> 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
> > > >>>>
> > > >>>
> > > >>
> > > >>
> > > >
> > >
> >
>

Re: Proposal: Add appId optional paramater to deploy api

[Duncan Grant <du...@cloudsoftcorp.com>]

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 <ge...@cloudsoft.io>
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 <
> alex.heneveld@cloudsoftcorp.com>
> 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 <al...@gmail.com> 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 <
> > duncan.grant@cloudsoftcorp.com>
> > >>>> 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
> > >>>>
> > >>>
> > >>
> > >>
> > >
> >
>

Re: Proposal: Add appId optional paramater to deploy api

[Geoff Macartney <ge...@cloudsoft.io>]

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 <al...@cloudsoftcorp.com>
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 <al...@gmail.com> 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 <
> duncan.grant@cloudsoftcorp.com>
> >>>> 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
> >>>>
> >>>
> >>
> >>
> >
>

Re: Proposal: Add appId optional paramater to deploy api

[Alex Heneveld <al...@cloudsoftcorp.com>]

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 <al...@gmail.com> 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 <du...@cloudsoftcorp.com>
>>>> 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
>>>>
>>>
>>
>>
>

Re: Proposal: Add appId optional paramater to deploy api

[Aled Sage <al...@gmail.com>]

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/55eb11fa91e9091447d56bb45116ccc3dc6009df


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 
>>> <du...@cloudsoftcorp.com> 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
>
>

Re: Proposal: Add appId optional paramater to deploy api

[Aled Sage <al...@gmail.com>]

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 <du...@cloudsoftcorp.com> 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

Re: Proposal: Add appId optional paramater to deploy api

[Svetoslav Neykov <sv...@cloudsoft.io>]

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 <du...@cloudsoftcorp.com> 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

Re: Proposal: Add appId optional paramater to deploy api

[Alex Heneveld <al...@cloudsoftcorp.com>]

good idea.  however the application's ID is not meant to be 
user-supplied.  maybe this could be called `deploymentId` and set 
(compare against) a config key called `deploymentId` ?

--a


On 07/07/2017 16:33, Duncan Grant 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
>