Re: Proposal: Add appId optional paramater to deploy api

2017-07-27 Thread Aled Sage
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

Re: Proposal: Add appId optional paramater to deploy api

2017-07-27 Thread Alex Heneveld
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.

Re: Proposal: Add appId optional paramater to deploy api

2017-07-26 Thread Aled Sage
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 prop

Re: Proposal: Add appId optional paramater to deploy api

2017-07-26 Thread Alex Heneveld
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

Re: Proposal: Add appId optional paramater to deploy api

2017-07-26 Thread Aled Sage
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 compell

Re: Proposal: Add appId optional paramater to deploy api

2017-07-26 Thread Alex Heneveld
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" wrote: Hi Mark, We removed from EntitySpec the abi

Re: Proposal: Add appId optional paramater to deploy api

2017-07-26 Thread Aled Sage
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 re

Re: Proposal: Add appId optional paramater to deploy api

2017-07-26 Thread Mark Mc Kenna
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,}

Re: Proposal: Add appId optional paramater to deploy api

2017-07-26 Thread Duncan Grant
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 o

Re: Proposal: Add appId optional paramater to deploy api

2017-07-26 Thread Geoff Macartney
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

Re: Proposal: Add appId optional paramater to deploy api

2017-07-26 Thread Aled Sage
// 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 sys

Re: Proposal: Add appId optional paramater to deploy api

2017-07-26 Thread Svetoslav Neykov
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

Re: Proposal: Add appId optional paramater to deploy api

2017-07-25 Thread Alex Heneveld
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

Re: Proposal: Add appId optional paramater to deploy api

2017-07-25 Thread Aled Sage
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

Re: Proposal: Add appId optional paramater to deploy api

2017-07-07 Thread Aled Sage
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

Re: Proposal: Add appId optional paramater to deploy api

2017-07-07 Thread Svetoslav Neykov
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 retur

Re: Proposal: Add appId optional paramater to deploy api

2017-07-07 Thread Alex Heneveld
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 endp

Proposal: Add appId optional paramater to deploy api

2017-07-07 Thread Duncan Grant
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 deploymen