[Ansible-service-broker] Deprovision default behaviour

[Philip Gough]

Sure, I'll get something going asap.

Philip

On Wed, Feb 14, 2018 at 5:09 PM, Ryan Hallisey <rhallise at redhat.com> wrote:

> > So while sure adding this flag seems to fix one aspect of it, albeit a
> > key aspect, it would be better to see a proposal on dev experience from
> > a developer perspective. And try to address the issues outlined and
> > discussed. That discussion might lead to this exact patch, but it will
> > be seen in the broader picture of how to fix the dev experience.
> >
> Sounds good, let's go with a broader dev proposal.  Philip, is that
> something you would be wiling to start? Here's a link to the template
> https://github.com/openshift/ansible-service-broker/blob/
> master/docs/proposals/proposal-template.md
>
> -Ryan
>
> On Wed, Feb 14, 2018 at 11:49 AM, jesus m. rodriguez <jesusr at redhat.com>
> wrote:
>
>> On Wed, 2018-02-14 at 11:29 -0500, Ryan Hallisey wrote:
>> > > > I don’t know if I feel comfortable with adding this logic to the
>> > > > primary
>> > > > code path for deprovision. This is mostly me being conservative
>> > > > with
>> >
>> > adding
>> > > > configuration values that can allow things to get deleted from
>> > > > our
>> >
>> > storage.
>> > > >
>> > >
>> > > The developer experience is not acceptable right now, but I have to
>> > > agree with Shawn.
>> > >
>> >
>> > Agreed, the dev experience is really rough with deprovision. For
>> > instance,
>> > if you are testing your provision playbook with the broker and your
>> > deprovision playbook fails, then you can't test provision again
>> > without
>> > some manually steps.  But, I think Phillip's patch is a good way to
>> > provide
>> > significant improvement to the dev experience with minimal code
>> > changes.
>> > The meat of his code is 4 lines:
>> >
>> > ```go
>> > if a.brokerConfig.ForceDelete {
>> >   log.Infof("Deprovision failed. Attempting to clean up related
>> > resources.
>> > User should ensure clean up is successful")
>> >   cleanupDeprovision(&instance, a.dao) // Return a 200 to the
>> > service-catlog
>> > }
>> > ```
>>
>> I think the problem is not necessarily that it is a seemingly small
>> patch. But that there is an overall bigger issue that the dev
>> experience isn't completely thought out yet.
>>
>> So while sure adding this flag seems to fix one aspect of it, albeit a
>> key aspect, it would be better to see a proposal on dev experience from
>> a developer perspective. And try to address the issues outlined and
>> discussed. That discussion might lead to this exact patch, but it will
>> be seen in the broader picture of how to fix the dev experience.
>>
>> My hesitation with applying this right now is I'd rather see what the
>> broader list of dev experience problems are. How we can address them?
>> Do we have a single flag that enables a bunch of things? Do we have a
>> set of dev flags that can be flipped for individual things? Etc.
>>
>> We already have a dev_broker flag. Now this patch is adding a
>> force_delete flag. While it is to fix a dev experience problem, how do
>> I know that flag is just for dev? What are the implications of enabling
>> said flag in a production environment? It's the list of subtleties that
>> I'd like to at least see or anticipate before addressing just one
>> aspect of a problem.
>>
>> >
>> > > Unless I missed it, let's backup and describe the high-level
>> > > problem
>> > > in a proposal and
>> > > submit solutions there. That's been our approach thus far and I
>> > > think
>> > > it applies here well.
>> >
>> > To make this the default behaviour, I agree needs a proposal.
>>
>> I would rather have a proposal to discuss the dev experience and needs.
>> Then have a series of PRs to address the needs that come from that
>> proposal. Again this patch fixes only one aspect of the dev experience.
>>
>> jesus
>>
>> _______________________________________________
>> Ansible-service-broker mailing list
>> Ansible-service-broker at redhat.com
>> https://www.redhat.com/mailman/listinfo/ansible-service-broker
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/ansible-service-broker/attachments/20180214/d3d02d7c/attachment.htm>