After some consideration and seeing the actual move in practice I wanted to propose 3rd amendment ;) to the AIP-21. I have a few observations from seeing the discussions and observing the actual moving process. I have the following proposals:
*1) Between-providers transfer operators should be kept at the "target" rather than "source"* If we end up with splitting operators by groups (AIP-8 and the proposed Backporting to Airflow 1.10), I think it makes more sense to keep transfer operators in the "target" package. For example "S3 to GCS" operator in "providers/google" package - simply because it is more likely that the individuals that will be working on the pure "GCP" services will also be more interested in getting the data from other cloud providers, and likely they will even have some transfer services that can be used for that purpose (rather than using worker to transfer the data) - in the particular S3-> GCS case we have GCP's https://cloud.google.com/storage-transfer/docs/overview which allows to transfer data from any other cloud provider to GCS . Same for example if we imagine Athena -> Bigquery for example. At least that's the feeling I have. I can imagine that the kind of "stewardship" over those groups of operators can be somewhat influenced and maybe even performed by those cloud providers themselves. Corresponding hooks of course should be in different "groups". 2) *One-side provider-neutral transfer operators should be kept at the "provider" regardless if they are target or source.* For example GCS-> SFTP or SFTP -> GCS. There the hook for SFTP should be in the "core" package but both operators should be in "providers/google". The reason is quite the same as above - the "stewardship" over all the operators can be done by the "provider" group. *3) Grouping non-provider operators/hooks according to their purpose.* I think it is also the right time to move the other operators/hooks to different groups within core. We already have some reasonable and nice groups proposed in the new documentation by Kamil https://airflow.readthedocs.io/en/latest/operators-and-hooks-ref.html and it only makes sense to move those now (Fundamentals, ASF: Apache Software Foundation, Azure: Microsoft Azure, AWS: Amazon Web Services, GCP: Google Cloud Platform, Service integrations, Software integrations, Protocol integrations). I think it would make sense to use the same approach in the code: We could have fundamentals/asf/azure(microsoft/azure?)/aws(amazon/aws?)/google/services/software/protocols) packages. There will be few exceptions probably but we can handle them on case-by-case basis. J. On Fri, Oct 11, 2019 at 3:11 PM Jarek Potiuk <jarek.pot...@polidea.com> wrote: > Hello everyone. I updated AIP-21 and updated examples. > > > Point D. of AIP-21 is now as follows: > > > > *D. * Group operators/sensors/hooks in > *airflow/providers/<PROVIDER>*/operators(sensors, > hooks). > > Each provider can define its own internal structure of that package. For > example in case of "google" provider the packages will be further grouped > by "gcp", "gsuite", "core" sub-packages. > > In case of transfer operators where two providers are involved, the > transfer operators will be moved to "source" of the transfer. When there > is only one provider as target but source is a database or another > non-provider source, the operator is put to the target provider. > > Non-cloud provider ones are moved to airflow/operators(sensors/hooks). > *Drop the prefix.* > > Examples: > > AWS operator: > > - > *airflow/contrib/operators/sns_publish_operator.py > becomes airflow/providers/aws/operators/**sns_publish_operator.py* > > *Google GCP operator:* > > - *airflow/contrib/operators/dataproc_operator.py* > becomes *airflow/providers/gooogle/gcp/operators/dataproc_operator.py* > > Previously GCP-prefixed operator: > > - > *airflow/contrib/operators/gcp_bigtable_operator.py *becomes > *airflow/providers/google/**gcp/operators/bigtable_operator.py* > > *Transfer from GCP:* > > - *airflow/contrib/operators/gcs_to_s3_operator.py* > * becomes airflow/providers/google/gcp/operators/gcs_to_s3_operator.py* > > *MySQL to GCS:* > > - *airflow/contrib/operators/mysql_to_gcs_operator.py* > * becomes airflow/providers/google/gcp/operators/* > *mysql_to_gcs_operator.py* > > *SSH operator:* > > - > *airflow/contrib/operators/ssh_operator.py *becomes *airflow/* > *operators/ssh_operator.py* > > > On Fri, Oct 4, 2019 at 6:22 PM Jarek Potiuk <jarek.pot...@polidea.com> > wrote: > >> Yeah. I think the important point is that the latest doc changes by Kamil >> index all available operators and hooks nicely and make them easy to find. >> >> That also includes (as of today) automated CI checking if new operators >> and hooks added are added to the documentation : >> https://github.com/apache/airflow/commit/104a151d6a19b1ba1281cb00c66a2c3409e1bb13 >> >> J. >> >> On Fri, Oct 4, 2019 at 5:21 PM Chris Palmer <ch...@crpalmer.com> wrote: >> >>> It's not obvious to me why an S3ToMsSQLOperator in the aws package is >>> "silly". Why do you say it made sense to create a MsSqlFromS3Operator? >>> >>> Basically all of these operators could be thought of as "move data from A >>> to B" or "move data to B from A". I think what feels natural to each >>> individual will depend on what their frame of reference is, and where >>> their >>> main focus is. If you are largely focused on MsSql then I can understand >>> that it's natural to think "What MsSql operators are there?" and to >>> not see S3ToMsSqlOperator >>> as one of those MsSql operators. That's exactly the point I made with my >>> earlier response; I was so focused on BigQuery that I didn't think to >>> look >>> under Cloud Storage documentation for the >>> GoogleCloudStorageToBigQueryOperator. >>> >>> I think it is too hard to draw a very distinct line between what is just >>> "storage" and what is more. There are going to be fuzzy edge cases, so >>> picking a single convention is going to much less hassle in my view. As >>> long as that convention is well documented and the documentation is >>> improved so that it's easier to find all operators that relate to >>> BigQuery >>> or MsSql etc in one place (as is being done by Kamil) then that is the >>> best >>> we can do. >>> >>> Chris >>> >>> >>> >>> On Fri, Oct 4, 2019 at 10:55 AM Daniel Standish <dpstand...@gmail.com> >>> wrote: >>> >>> > One case popped up for us recently, where it made sense to make a MsSql >>> > *From*S3Operator . >>> > >>> > I think using "source" makes sense in general, but in this case calling >>> > this a S3ToMsSqlOperator and putting it under AWS seems silly, even >>> though >>> > you could say s3 is "source" here. >>> > >>> > I think in most of these cases we say "let's use source" because >>> source is >>> > where the actual work is done and destination is just storage. >>> > >>> > Does a guideline saying "ignore storage" or "storage is secondary in >>> object >>> > location" make sense? >>> > >>> > >>> > >>> > On Fri, Oct 4, 2019 at 6:42 AM Jarek Potiuk <jarek.pot...@polidea.com> >>> > wrote: >>> > >>> > > It looks like we have general consensus about putting transfer >>> operators >>> > > into "source provider" package. >>> > > That's great for me as well. >>> > > >>> > > Since I will be updating AIP-21 to reflect the "google" vs. "gcp" >>> case, I >>> > > will also update it to add this decision. >>> > > >>> > > If no-one objects (Lazy Consensus >>> > > <https://community.apache.org/committers/lazyConsensus.html>) till >>> > > Monday7th of October, 3.20 CEST, we will update AIP-21 with >>> information >>> > > that transfer operators should be placed in the "source" provider >>> module. >>> > > >>> > > J. >>> > > >>> > > On Tue, Sep 24, 2019 at 1:34 PM Kamil Breguła < >>> kamil.breg...@polidea.com >>> > > >>> > > wrote: >>> > > >>> > > > On Mon, Sep 23, 2019 at 7:42 PM Chris Palmer <ch...@crpalmer.com> >>> > wrote: >>> > > > > >>> > > > > On Mon, Sep 23, 2019 at 1:22 PM Kamil Breguła < >>> > > kamil.breg...@polidea.com >>> > > > > >>> > > > > wrote: >>> > > > > >>> > > > > > On Mon, Sep 23, 2019 at 7:04 PM Chris Palmer < >>> ch...@crpalmer.com> >>> > > > wrote: >>> > > > > > > >>> > > > > > > Is there a reason why we can't use symlinks to have copies >>> of the >>> > > > files >>> > > > > > > show up in both subpackages? So that `gcs_to_s3.py` would be >>> > under >>> > > > both >>> > > > > > > `aws/operators/` and `gcp/operators`. I could imagine there >>> may >>> > be >>> > > > > > > technical reasons why this is a bad idea, but just thought I >>> > would >>> > > > ask. >>> > > > > > > >>> > > > > > Symlinks is not supported by git. >>> > > > > > >>> > > > > > >>> > > > > Why do you say that? This blog post >>> > > > > <https://www.mokacoding.com/blog/symliks-in-git/> details how >>> you >>> > can >>> > > > use >>> > > > > them, and the caveats with regards to needing relative links not >>> > > > absolute. >>> > > > > The example repo he links to at the end includes a symlink which >>> > worked >>> > > > > fine for me when I cloned it. But maybe not relevant given the >>> below: >>> > > > >>> > > > We still have to check if python packages can have links, but I'm >>> > > > afraid of this mechanism. This is not popular and may cause >>> unexpected >>> > > > consequences. >>> > > > >>> > > > >>> > > > > > > Likewise, someone who spends 99% of their time working in >>> AWS and >>> > > > using >>> > > > > > all >>> > > > > > > the operators in that subpackage, might not think to look in >>> the >>> > > GCP >>> > > > > > > package the first time they need a GCS to S3 operator. I'm >>> > > admittedly >>> > > > > > > terrible at documentation, but if duplicating the files via >>> > > symlinks >>> > > > > > isn't >>> > > > > > > an option, then is there an easy way we could duplicate the >>> > > > documentation >>> > > > > > > for those operators so they are easily findable in both doc >>> > > sections? >>> > > > > > > >>> > > > > > >>> > > > > > Recently, I updated the documentation: >>> > > > > > https://airflow.readthedocs.io/en/latest/integration.html >>> > > > > > We have list of all integration in AWS, Azure, GCP. If the >>> > operator >>> > > > > > concerns two cloud proivders, it repeats in two places. It's >>> good >>> > for >>> > > > > > documentation. DRY rule is only valid for source code. >>> > > > > > I am working on documentation for other operators. >>> > > > > > My work is part of this ticket: >>> > > > > > https://issues.apache.org/jira/browse/AIRFLOW-5431 >>> > > > > > >>> > > > > > >>> > > > > This updated documentation looks great, definitely heading in a >>> > > direction >>> > > > > that makes it easier and addresses my concerns. (Although it >>> took me >>> > a >>> > > > > while to realize those tables can be scrolled horizontally!). >>> > > > > >>> > > > I'm working on redesign of documentation theme. It's part of AIP-11 >>> > > > >>> > > > >>> > > >>> > >>> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-11+Create+a+Landing+Page+for+Apache+Airflow >>> > > > We are currently at the stage of collecting comments from the first >>> > > > phase - we sent materials to the community, but also conducted >>> tests >>> > > > with real users >>> > > > >>> > > > >>> > > >>> > >>> https://lists.apache.org/thread.html/6fa1cdceb97ed17752978a8d4202bf1ff1a86c6b50bbc9d09f694166@%3Cdev.airflow.apache.org%3E >>> > > > >>> > > >>> > > >>> > > -- >>> > > >>> > > Jarek Potiuk >>> > > Polidea <https://www.polidea.com/> | Principal Software Engineer >>> > > >>> > > M: +48 660 796 129 <+48660796129> >>> > > [image: Polidea] <https://www.polidea.com/> >>> > > >>> > >>> >> >> >> -- >> >> Jarek Potiuk >> Polidea <https://www.polidea.com/> | Principal Software Engineer >> >> M: +48 660 796 129 <+48660796129> >> [image: Polidea] <https://www.polidea.com/> >> >> > > -- > > Jarek Potiuk > Polidea <https://www.polidea.com/> | Principal Software Engineer > > M: +48 660 796 129 <+48660796129> > [image: Polidea] <https://www.polidea.com/> > > -- Jarek Potiuk Polidea <https://www.polidea.com/> | Principal Software Engineer M: +48 660 796 129 <+48660796129> [image: Polidea] <https://www.polidea.com/>
