search

astronomer / apache-airflow-providers-transfers

https://apache-airflow-provider-transfers.rtfd.io/
Apache License 2.0
11 stars 3 forks source link

Add logic to convert dict to Options class #55

Closed utkarsharma2 closed 1 year ago

utkarsharma2 commented 1 year ago

Description

What is the current behavior?

Currently, for transfer_params we have a logic to convert dict to TransferIntegrationOptions but that doesn't seem to be executed. Due to this, we are essentially using dict type in our codebase assuming it's of type TransferIntegrationOptions. Which seems to mess with static type check.

closes: https://github.com/astronomer/apache-airflow-provider-transfers/issues/51

What is the new behavior?

We need to use the TransferIntegrationOptions class or its subclass internally, so any dict that is passed to transfer_params should be converted to TransferIntegrationOptions or a subclass.

Algorithm:

  1. If users have passed a TransferIntegrationOptions or subclass of it. We let it pass, assuming the user knows what he is doing.
  2. If the user have passed a dict, convert it to an Option class, We check if the transfer mode is third-party,
    1. Third-party - Then the logic for getting the correct Ingestor class comes from conn_id passed in transfer_param. Once we get the Ingestor class we look for the OPTIONS_CLASS attribute, this attribute is supposed to hold a reference to correct Options Class
      1. Attribute found - We dict to instantiate this class.
      2. Attribute not found - We use the class TransferIntegrationOptions and instantiate it with dict.
    2. Native - Then the logic for getting the correct options class goes to provider of destination_dataset. we determineprovider class by conn_id of destination_dataset. Once we have the provider class, we look for OPTIONS_CLASS attribute, this attribute is supposed to hold a reference to correct OPTIONS_CLASS
      1. OPTIONS_CLASS Attribute found - We use dict to instantiate this class.
      2. OPTIONS_CLASS Attribute not found - We use the class TransferIntegrationOptions and instantiate it with dict.

Does this introduce a breaking change?

Nope

Checklist

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 64.06% and project coverage change: +0.11 :tada:

Comparison is base (5a81abb) 66.51% compared to head (1be7369) 66.62%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #55 +/- ## ========================================== + Coverage 66.51% 66.62% +0.11% ========================================== Files 32 32 Lines 1723 1753 +30 Branches 165 157 -8 ========================================== + Hits 1146 1168 +22 - Misses 530 548 +18 + Partials 47 37 -10 ``` | Flag | Coverage Δ | | |---|---|---| | UTO | `66.62% <64.06%> (+0.11%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astronomer#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/astronomer/apache-airflow-provider-transfers/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astronomer) | Coverage Δ | | |---|---|---| | [...\_transfer\_operator/data\_providers/database/base.py](https://codecov.io/gh/astronomer/apache-airflow-provider-transfers/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astronomer#diff-c3JjL3VuaXZlcnNhbF90cmFuc2Zlcl9vcGVyYXRvci9kYXRhX3Byb3ZpZGVycy9kYXRhYmFzZS9iYXNlLnB5) | `78.15% <ø> (-0.59%)` | :arrow_down: | | [...perator/data\_providers/database/google/bigquery.py](https://codecov.io/gh/astronomer/apache-airflow-provider-transfers/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astronomer#diff-c3JjL3VuaXZlcnNhbF90cmFuc2Zlcl9vcGVyYXRvci9kYXRhX3Byb3ZpZGVycy9kYXRhYmFzZS9nb29nbGUvYmlncXVlcnkucHk=) | `72.15% <ø> (+0.90%)` | :arrow_up: | | [...ransfer\_operator/data\_providers/database/sqlite.py](https://codecov.io/gh/astronomer/apache-airflow-provider-transfers/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astronomer#diff-c3JjL3VuaXZlcnNhbF90cmFuc2Zlcl9vcGVyYXRvci9kYXRhX3Byb3ZpZGVycy9kYXRhYmFzZS9zcWxpdGUucHk=) | `81.48% <ø> (+1.48%)` | :arrow_up: | | [...ransfer\_operator/data\_providers/filesystem/base.py](https://codecov.io/gh/astronomer/apache-airflow-provider-transfers/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astronomer#diff-c3JjL3VuaXZlcnNhbF90cmFuc2Zlcl9vcGVyYXRvci9kYXRhX3Byb3ZpZGVycy9maWxlc3lzdGVtL2Jhc2UucHk=) | `82.88% <ø> (+0.90%)` | :arrow_up: | | [...ransfer\_operator/data\_providers/filesystem/sftp.py](https://codecov.io/gh/astronomer/apache-airflow-provider-transfers/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astronomer#diff-c3JjL3VuaXZlcnNhbF90cmFuc2Zlcl9vcGVyYXRvci9kYXRhX3Byb3ZpZGVycy9maWxlc3lzdGVtL3NmdHAucHk=) | `86.95% <ø> (+0.93%)` | :arrow_up: | | [...iversal\_transfer\_operator/integrations/fivetran.py](https://codecov.io/gh/astronomer/apache-airflow-provider-transfers/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astronomer#diff-c3JjL3VuaXZlcnNhbF90cmFuc2Zlcl9vcGVyYXRvci9pbnRlZ3JhdGlvbnMvZml2ZXRyYW4ucHk=) | `0.00% <0.00%> (ø)` | | | [...iversal\_transfer\_operator/integrations/\_\_init\_\_.py](https://codecov.io/gh/astronomer/apache-airflow-provider-transfers/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astronomer#diff-c3JjL3VuaXZlcnNhbF90cmFuc2Zlcl9vcGVyYXRvci9pbnRlZ3JhdGlvbnMvX19pbml0X18ucHk=) | `40.00% <27.77%> (+1.11%)` | :arrow_up: | | [...fer\_operator/data\_providers/filesystem/\_\_init\_\_.py](https://codecov.io/gh/astronomer/apache-airflow-provider-transfers/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astronomer#diff-c3JjL3VuaXZlcnNhbF90cmFuc2Zlcl9vcGVyYXRvci9kYXRhX3Byb3ZpZGVycy9maWxlc3lzdGVtL19faW5pdF9fLnB5) | `73.68% <50.00%> (ø)` | | | [...l\_transfer\_operator/universal\_transfer\_operator.py](https://codecov.io/gh/astronomer/apache-airflow-provider-transfers/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astronomer#diff-c3JjL3VuaXZlcnNhbF90cmFuc2Zlcl9vcGVyYXRvci91bml2ZXJzYWxfdHJhbnNmZXJfb3BlcmF0b3IucHk=) | `58.33% <50.00%> (-5.96%)` | :arrow_down: | | [...ersal\_transfer\_operator/data\_providers/\_\_init\_\_.py](https://codecov.io/gh/astronomer/apache-airflow-provider-transfers/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astronomer#diff-c3JjL3VuaXZlcnNhbF90cmFuc2Zlcl9vcGVyYXRvci9kYXRhX3Byb3ZpZGVycy9fX2luaXRfXy5weQ==) | `92.85% <88.23%> (-7.15%)` | :arrow_down: | | ... and [4 more](https://codecov.io/gh/astronomer/apache-airflow-provider-transfers/pull/55?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astronomer) | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astronomer). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=astronomer)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

utkarsharma2 commented 1 year ago

@sunank200 Addressed your concern, please take a look.