You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/10/11 18:33:33 UTC
[GitHub] [airflow] potix2 opened a new pull request #11434: Type annotation for aws operators in transfers
potix2 opened a new pull request #11434:
URL: https://github.com/apache/airflow/pull/11434
This PR is about increasing typing coverage for the aws provider. Part of: #9708
I annotated types for aws operators under `transfers` directory.
The typing coverage of amazon provider increases 45.8 to 55.6.
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#issuecomment-707947737
Hey @potix2 . Can you please rebase this one to the latest master. We fixed (hopefully) a problem with queues of jobs for GitHub actions and I think when you rebase, it should run much faster (more info on devlist shortly).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#discussion_r502968374
##########
File path: airflow/providers/amazon/aws/transfers/hive_to_dynamodb.py
##########
@@ -64,18 +65,18 @@ class HiveToDynamoDBOperator(BaseOperator):
def __init__( # pylint: disable=too-many-arguments
self,
*,
- sql,
- table_name,
- table_keys,
- pre_process=None,
- pre_process_args=None,
- pre_process_kwargs=None,
- region_name=None,
- schema='default',
- hiveserver2_conn_id='hiveserver2_default',
- aws_conn_id='aws_default',
+ sql: str,
+ table_name: str,
+ table_keys: list,
+ pre_process: Optional[Callable[[Any, Optional[list], Optional[list]], Iterable]] = None,
Review comment:
```suggestion
pre_process: Optional[Callable] = None,
```
No strong opinion but this long type is too much complicated for me
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potix2 commented on pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
potix2 commented on pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#issuecomment-707310502
Thank you. I installed pre-commit and fixed failures.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potix2 commented on a change in pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
potix2 commented on a change in pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#discussion_r504358921
##########
File path: airflow/providers/google/cloud/operators/bigquery.py
##########
@@ -947,9 +947,7 @@ def execute(self, context) -> None:
delegate_to=self.delegate_to,
impersonation_chain=self.impersonation_chain,
)
- schema_fields = json.loads(
- gcs_hook.download(gcs_bucket, gcs_object).decode("utf-8") # type: ignore[attr-defined]
- ) # type: ignore[attr-defined]
+ schema_fields = json.loads(gcs_hook.download(gcs_bucket, gcs_object))
Review comment:
I got an error in CI. To fix it this modification is necessary. In this case GCSHook.download() returns always `bytes` and json.loads() calls decode internally.
If we preserve the call to `decode`, we need to cast the returned value from `download` to bytes. @turbaszek could you advise me about this point?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#discussion_r502968122
##########
File path: airflow/providers/amazon/aws/transfers/google_api_to_s3.py
##########
@@ -167,21 +167,21 @@ def _retrieve_data_from_google_api(self):
)
return google_api_response
- def _load_data_to_s3(self, data):
+ def _load_data_to_s3(self, data: dict) -> None:
s3_hook = S3Hook(aws_conn_id=self.aws_conn_id)
s3_hook.load_string(
string_data=json.dumps(data), key=self.s3_destination_key, replace=self.s3_overwrite
)
- def _update_google_api_endpoint_params_via_xcom(self, task_instance):
+ def _update_google_api_endpoint_params_via_xcom(self, task_instance: TaskInstance) -> None:
google_api_endpoint_params = task_instance.xcom_pull(
task_ids=self.google_api_endpoint_params_via_xcom_task_ids,
key=self.google_api_endpoint_params_via_xcom,
)
self.google_api_endpoint_params.update(google_api_endpoint_params)
- def _expose_google_api_response_via_xcom(self, task_instance, data):
+ def _expose_google_api_response_via_xcom(self, task_instance: TaskInstance, key: str, data: dict) -> None:
if sys.getsizeof(data) < MAX_XCOM_SIZE:
- task_instance.xcom_push(key=self.google_api_response_via_xcom, value=data)
+ task_instance.xcom_push(key=key, value=data)
Review comment:
How about adding
```
key = self.google_api_response_via_xcom or XCOM_RETURN_KEY
```
between L151 and L152?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek merged pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
turbaszek merged pull request #11434:
URL: https://github.com/apache/airflow/pull/11434
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#discussion_r506633993
##########
File path: airflow/providers/google/cloud/operators/bigquery.py
##########
@@ -947,9 +947,7 @@ def execute(self, context) -> None:
delegate_to=self.delegate_to,
impersonation_chain=self.impersonation_chain,
)
- schema_fields = json.loads(
- gcs_hook.download(gcs_bucket, gcs_object).decode("utf-8") # type: ignore[attr-defined]
- ) # type: ignore[attr-defined]
+ schema_fields = json.loads(gcs_hook.download(gcs_bucket, gcs_object))
Review comment:
Looks good to me. I'm surprised we didn't catch it earlier
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#issuecomment-707306455
[The Workflow run](https://github.com/apache/airflow/actions/runs/302822872) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potix2 commented on a change in pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
potix2 commented on a change in pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#discussion_r502990795
##########
File path: airflow/providers/amazon/aws/transfers/hive_to_dynamodb.py
##########
@@ -64,18 +65,18 @@ class HiveToDynamoDBOperator(BaseOperator):
def __init__( # pylint: disable=too-many-arguments
self,
*,
- sql,
- table_name,
- table_keys,
- pre_process=None,
- pre_process_args=None,
- pre_process_kwargs=None,
- region_name=None,
- schema='default',
- hiveserver2_conn_id='hiveserver2_default',
- aws_conn_id='aws_default',
+ sql: str,
+ table_name: str,
+ table_keys: list,
+ pre_process: Optional[Callable[[Any, Optional[list], Optional[list]], Iterable]] = None,
Review comment:
Thank you for your suggestion. I'll fix it!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#issuecomment-707308537
It seems that CI is sad because static code checks are failing. You may find this useful: https://github.com/apache/airflow/blob/master/STATIC_CODE_CHECKS.rst
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potix2 commented on a change in pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
potix2 commented on a change in pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#discussion_r504024870
##########
File path: airflow/providers/amazon/aws/transfers/gcs_to_s3.py
##########
@@ -183,7 +183,7 @@ def execute(self, context):
self.log.info("Saving file to %s", dest_key)
s3_hook.load_bytes(
- file_bytes, key=dest_key, replace=self.replace, acl_policy=self.s3_acl_policy
+ file_bytes.encode(), key=dest_key, replace=self.replace, acl_policy=self.s3_acl_policy
Review comment:
S3Hook.load_bytes() requires `bytes` but `file_bytes` is string, so I call encode() to convert string to bytes. The reason why some tests got error in CI might be this modification.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potix2 commented on a change in pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
potix2 commented on a change in pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#discussion_r503495322
##########
File path: airflow/providers/amazon/aws/transfers/dynamodb_to_s3.py
##########
@@ -111,7 +111,7 @@ def __init__(
self.s3_bucket_name = s3_bucket_name
self.s3_key_prefix = s3_key_prefix
- def execute(self, context):
+ def execute(self, context: Dict[str, Any]) -> None:
Review comment:
I got it. I removed some redundant type hints for context.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#issuecomment-707341674
[The Workflow run](https://github.com/apache/airflow/actions/runs/302896614) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#discussion_r502968228
##########
File path: airflow/providers/amazon/aws/transfers/google_api_to_s3.py
##########
@@ -167,21 +167,21 @@ def _retrieve_data_from_google_api(self):
)
return google_api_response
- def _load_data_to_s3(self, data):
+ def _load_data_to_s3(self, data: dict) -> None:
s3_hook = S3Hook(aws_conn_id=self.aws_conn_id)
s3_hook.load_string(
string_data=json.dumps(data), key=self.s3_destination_key, replace=self.s3_overwrite
)
- def _update_google_api_endpoint_params_via_xcom(self, task_instance):
+ def _update_google_api_endpoint_params_via_xcom(self, task_instance: TaskInstance) -> None:
google_api_endpoint_params = task_instance.xcom_pull(
task_ids=self.google_api_endpoint_params_via_xcom_task_ids,
key=self.google_api_endpoint_params_via_xcom,
)
self.google_api_endpoint_params.update(google_api_endpoint_params)
- def _expose_google_api_response_via_xcom(self, task_instance, data):
+ def _expose_google_api_response_via_xcom(self, task_instance: TaskInstance, key: str, data: dict) -> None:
if sys.getsizeof(data) < MAX_XCOM_SIZE:
- task_instance.xcom_push(key=self.google_api_response_via_xcom, value=data)
+ task_instance.xcom_push(key=key, value=data)
Review comment:
How about doing:
```
key = self.google_api_response_via_xcom or XCOM_RETURN_KEY
```
instead of changing the signature?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potix2 commented on a change in pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
potix2 commented on a change in pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#discussion_r502954249
##########
File path: airflow/providers/amazon/aws/hooks/base_aws.py
##########
@@ -393,7 +394,7 @@ def get_session(self, region_name: Optional[str] = None) -> boto3.session.Sessio
session, _ = self._get_credentials(region_name)
return session
- def get_credentials(self, region_name: Optional[str] = None) -> Tuple[Optional[str], Optional[str]]:
Review comment:
The `botocore.credentials.ReadOnlyCredentials` is namedtuple. It's not tuple. To avoid typing error in `s3_to_redshift.py` this modification is necessary.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potix2 commented on a change in pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
potix2 commented on a change in pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#discussion_r502952746
##########
File path: airflow/providers/amazon/aws/transfers/s3_to_redshift.py
##########
@@ -87,13 +87,11 @@ def __init__(
self.verify = verify
self.copy_options = copy_options or []
self.autocommit = autocommit
- self._s3_hook = None
- self._postgres_hook = None
- def execute(self, context):
- self._postgres_hook = PostgresHook(postgres_conn_id=self.redshift_conn_id)
- self._s3_hook = S3Hook(aws_conn_id=self.aws_conn_id, verify=self.verify)
- credentials = self._s3_hook.get_credentials()
+ def execute(self, context: Dict[str, Any]) -> None:
+ postgres_hook = PostgresHook(postgres_conn_id=self.redshift_conn_id)
Review comment:
These variables are referenced from only `execute()`, so I changed scope of them. If we keep them as the attributes of the instance, their type are Optional[T]. I think it's a little redundant, because they have always actual values in execute().
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#issuecomment-710313987
Awesome work, congrats on your first merged pull request!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potix2 commented on a change in pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
potix2 commented on a change in pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#discussion_r504059360
##########
File path: airflow/providers/amazon/aws/transfers/gcs_to_s3.py
##########
@@ -183,7 +183,7 @@ def execute(self, context):
self.log.info("Saving file to %s", dest_key)
s3_hook.load_bytes(
- file_bytes, key=dest_key, replace=self.replace, acl_policy=self.s3_acl_policy
+ file_bytes.encode(), key=dest_key, replace=self.replace, acl_policy=self.s3_acl_policy
Review comment:
The return type of GCSHook.download() is currently `str` but it might be `Union[str, bytes]`. When `filename` is set, it returns `filename` in another case it returns content of downloaded file as `bytes`. So I'll remove `encode()` from here and modify the return type of GCSHook.download().
https://github.com/apache/airflow/blob/master/airflow/providers/google/cloud/hooks/gcs.py#L263
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#issuecomment-706747887
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, pylint and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/master/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/master/docs/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/master/BREEZE.rst) for testing locally, itβs a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#coding-style-and-best-practices).
Apache Airflow is a community-driven project and together we are making it better π.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potix2 commented on a change in pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
potix2 commented on a change in pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#discussion_r502953702
##########
File path: airflow/providers/amazon/aws/transfers/google_api_to_s3.py
##########
@@ -167,21 +167,21 @@ def _retrieve_data_from_google_api(self):
)
return google_api_response
- def _load_data_to_s3(self, data):
+ def _load_data_to_s3(self, data: dict) -> None:
s3_hook = S3Hook(aws_conn_id=self.aws_conn_id)
s3_hook.load_string(
string_data=json.dumps(data), key=self.s3_destination_key, replace=self.s3_overwrite
)
- def _update_google_api_endpoint_params_via_xcom(self, task_instance):
+ def _update_google_api_endpoint_params_via_xcom(self, task_instance: TaskInstance) -> None:
google_api_endpoint_params = task_instance.xcom_pull(
task_ids=self.google_api_endpoint_params_via_xcom_task_ids,
key=self.google_api_endpoint_params_via_xcom,
)
self.google_api_endpoint_params.update(google_api_endpoint_params)
- def _expose_google_api_response_via_xcom(self, task_instance, data):
+ def _expose_google_api_response_via_xcom(self, task_instance: TaskInstance, key: str, data: dict) -> None:
if sys.getsizeof(data) < MAX_XCOM_SIZE:
- task_instance.xcom_push(key=self.google_api_response_via_xcom, value=data)
+ task_instance.xcom_push(key=key, value=data)
Review comment:
The argument `key` of `xcom_push()` method requires `str` value but `self.google_api_response_via_xcom` is `Optional[str]` so we need to check that it's not None. In the outer scope of this method it's checked already, so I change the signature of this method to receive it from the outer method.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potix2 commented on pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
potix2 commented on pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#issuecomment-707519725
@turbaszek I rebased this branch and fixed all failures of static checking in CI. Could you review this again?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potix2 commented on a change in pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
potix2 commented on a change in pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#discussion_r506678114
##########
File path: airflow/providers/google/cloud/operators/bigquery.py
##########
@@ -947,9 +947,7 @@ def execute(self, context) -> None:
delegate_to=self.delegate_to,
impersonation_chain=self.impersonation_chain,
)
- schema_fields = json.loads(
- gcs_hook.download(gcs_bucket, gcs_object).decode("utf-8") # type: ignore[attr-defined]
- ) # type: ignore[attr-defined]
+ schema_fields = json.loads(gcs_hook.download(gcs_bucket, gcs_object))
Review comment:
Thank you for your review. Static type checking is really cool because we can find like above the issues.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potix2 commented on a change in pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
potix2 commented on a change in pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#discussion_r502992056
##########
File path: airflow/providers/amazon/aws/transfers/google_api_to_s3.py
##########
@@ -167,21 +167,21 @@ def _retrieve_data_from_google_api(self):
)
return google_api_response
- def _load_data_to_s3(self, data):
+ def _load_data_to_s3(self, data: dict) -> None:
s3_hook = S3Hook(aws_conn_id=self.aws_conn_id)
s3_hook.load_string(
string_data=json.dumps(data), key=self.s3_destination_key, replace=self.s3_overwrite
)
- def _update_google_api_endpoint_params_via_xcom(self, task_instance):
+ def _update_google_api_endpoint_params_via_xcom(self, task_instance: TaskInstance) -> None:
google_api_endpoint_params = task_instance.xcom_pull(
task_ids=self.google_api_endpoint_params_via_xcom_task_ids,
key=self.google_api_endpoint_params_via_xcom,
)
self.google_api_endpoint_params.update(google_api_endpoint_params)
- def _expose_google_api_response_via_xcom(self, task_instance, data):
+ def _expose_google_api_response_via_xcom(self, task_instance: TaskInstance, key: str, data: dict) -> None:
if sys.getsizeof(data) < MAX_XCOM_SIZE:
- task_instance.xcom_push(key=self.google_api_response_via_xcom, value=data)
+ task_instance.xcom_push(key=key, value=data)
Review comment:
Thank you. Using XCOM_RETURN_KEY is simpler than changing the signature. I'll fix it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#issuecomment-707341343
[The Workflow run](https://github.com/apache/airflow/actions/runs/302863049) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#discussion_r525989282
##########
File path: airflow/providers/google/cloud/operators/bigquery.py
##########
@@ -947,9 +947,7 @@ def execute(self, context) -> None:
delegate_to=self.delegate_to,
impersonation_chain=self.impersonation_chain,
)
- schema_fields = json.loads(
- gcs_hook.download(gcs_bucket, gcs_object).decode("utf-8") # type: ignore[attr-defined]
- ) # type: ignore[attr-defined]
+ schema_fields = json.loads(gcs_hook.download(gcs_bucket, gcs_object))
Review comment:
Well, this never worked as per #12439 π
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #11434: Type annotation for aws operators in transfers
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11434:
URL: https://github.com/apache/airflow/pull/11434#discussion_r502967894
##########
File path: airflow/providers/amazon/aws/transfers/dynamodb_to_s3.py
##########
@@ -111,7 +111,7 @@ def __init__(
self.s3_bucket_name = s3_bucket_name
self.s3_key_prefix = s3_key_prefix
- def execute(self, context):
+ def execute(self, context: Dict[str, Any]) -> None:
Review comment:
I think mypy may moan about this. We don't need to add it as it is in BaseOperator
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org