Also - why the MUST:

If you use custom fields, in the current implementation, when you have
both short and long form "available", when you edit such a connection,
the "extras" dictionary is overridden with long names (simply because
the JSON extra field is disabled and there is no "merge" logic for
handling both.

So there is a scenario where you have a "short" name defined in extra
initially - but when you edit the connection, it will disappear
(unless you manually fill the "form" field with the same value).

That's why for connections that implement "custom" field behaviours
the "short" form is really "deprecated" value (to support a case where
the user uses connection_uri or has a database configured with "short"
names from earlier manual "editing" the JSON field).

And yes - it's not ideal, but I think the only proper way to handle it
is to implement better behaviour of the UI and support for
"namespaced" per-connection short names (which should not be too
difficult for anyone with some flair for javascript + Flask App
Builder).
Until then, we have to live with the dichotomy, I am afraid.

J.

On Sun, Nov 21, 2021 at 7:18 PM Jarek Potiuk <[email protected]> wrote:
>
> The problem is that if we use short extras, until we fix that in the
> UI, customizations won't work or they might override each other for
> different connections.
>
> So my take on it is - (until we fix the UI):
>
> a) if the connection customizes UI fields (so that you DO NOT edit the
> extras as JSON in the UI "extras" field manually) - you MUST use the
> long form.
> b) If the connection does not customize UI fields (so that you have to
> edit extras as JSIN in the UI "extras" field manually) - it is
> preferred to use short form
>
> J.
>
> On Sun, Nov 21, 2021 at 7:09 PM Daniel Standish <[email protected]> wrote:
> >
> > I wasn't exactly proposing updating the front end to handle short names.  
> > It sounds like a reasonable idea but I am not trying to take that on right 
> > now.
> >
> > What I was trying to ask was, given the present form behavior, what should 
> > the hook behavior be, by convention?
> >
> > Specifically, should we write our conn parsing in hooks such that whenever 
> > there is a form extra (e.g. extra__google__hello) we always do 
> > `extra.get('hello') or extra.get('extra__google__hello')`
> >
> > Or something along those lines.
> >
> > Or should we stick with the status quo, which is to say there is no such 
> > convention and some hooks will accept both and others will not.
> >
> > I think my vote would be that by convention we should check both and short 
> > beats long in precedence.  This doesn't mean we have to go update every 
> > hook with form customizations.  But if we're reviewing a PR that checks 
> > both then it's ok to approve, and if we're reviewing a PR that only checks  
> > long name we can suggest checking short name also.
> >
> > WDYT?
> >
> >

Reply via email to