Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review connection/hook usage #12702

Closed
potiuk opened this issue Nov 29, 2020 · 3 comments
Closed

Review connection/hook usage #12702

potiuk opened this issue Nov 29, 2020 · 3 comments
Labels

Comments

@potiuk
Copy link
Member

potiuk commented Nov 29, 2020

Description

The discussion in #12466 (comment) had shown that there is likely an expectation about making sure all the connections are 'registrable' in the UI of Airflow. This is not the case currently. Not all connections are visible in the UI (and possibly for a good reason) and currently connections are defined by Hook class variables.

Only a subset of the existing hooks provides this information, first of all because many of the connections/hooks do not require any custom fields, behaviour, they do not use automated Hook creation by connection type, but also it is because many of the hooks share the same connection type (For example almost all Google Hooks, most Amazon hooks etc).

Seems that we have 1 <> Many relationships Connection/Hook rather than 1-1 as initially designed. Also, there is an existing "standard" introduced by the DBApi Hook where this metadata is stored in the Hook class as class-level field. Which we carried to those hooks that required to register themselves in the UI:

For example:

class ImapHook(BaseHook):
    conn_name_attr = 'imap_conn_id'
    default_conn_name = 'imap_default'
    conn_type = 'imap'
    conn_name = 'IMAP'

We likely need to address that by defining clear rules for the hooks/providers:

  • whether we want to be able to map all hooks to connections
  • whether we still want to create hook class automatically based on connection type - especially where we have multiple hooks sharing one connection type
  • should we maybe have connection aliases to alias same connection type for different hooks
  • where to keep that information (in Hooks ? Elsewehere)
  • how to name the connection types - especially when there are siblings and multiple hooks sharing connection type.

I think those questions need to be answered before we can think about starting any implementation.

Related Issues

#12466 #11429 #12465

@potiuk potiuk added the kind:feature Feature Requests label Nov 29, 2020
@potiuk potiuk added this to the Airflow 2.1 milestone Nov 29, 2020
@potiuk
Copy link
Member Author

potiuk commented Nov 29, 2020

cc: @mik-laj @ashb -> if you want to tackle this, I described the situation and open questions we have for the Hook/Connections story. Marked it provisionally for 2.1. Feel free to take a lead on it!

@kaxil
Copy link
Member

kaxil commented Sep 14, 2021

I think this is done now, right @potiuk ?

@kaxil kaxil removed this from the Airflow 2.2 milestone Sep 14, 2021
@potiuk
Copy link
Member Author

potiuk commented Sep 14, 2021

I think the problems about some inconsistent connection have been handled already. Part of them in separate provider changes and final step was done with Provider's managers refactoring and import optimisations. I think all the cases with 1<>Many relations which I was aware of have been handled - without setting clear rules though.

I think it is handled in a "good-enough" way for now. Any rule setting and enforcing them should be done when we migrate-away from WTforms and declarative way of defining connections in the UI with the new modern UI (or abandon the idea of configurable connections via UI, but that does not seem likely).

@potiuk potiuk closed this as completed Sep 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants