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

Fix DbApiHook.insert_rows when rows is a generator #38972

Merged
merged 2 commits into from
Apr 12, 2024

Conversation

vincbeck
Copy link
Contributor

#38715 introduced a bug when the parameter rows is a generator. In that case, the method raises this error:

ERROR [airflow.task] Task failed with exception
Traceback (most recent call last):
  File "/opt/airflow/airflow/models/taskinstance.py", line 478, in _execute_task
    result = _execute_callable(context=context, **execute_callable_kwargs)
  File "/opt/airflow/airflow/models/taskinstance.py", line 441, in _execute_callable
    return ExecutionCallableRunner(
  File "/opt/airflow/airflow/utils/operator_helpers.py", line 250, in run
    return self.func(*args, **kwargs)
  File "/opt/airflow/airflow/models/baseoperator.py", line 405, in wrapper
    return func(self, *args, **kwargs)
  File "/opt/airflow/airflow/providers/amazon/aws/transfers/s3_to_sql.py", line 115, in execute
    self.db_hook.insert_rows(
  File "/opt/airflow/airflow/providers/common/sql/hooks/sql.py", line 597, in insert_rows
    self.log.info("Done loading. Loaded a total of %s rows into %s", len(rows), table)
TypeError: object of type 'generator' has no len()

Our system test example_s3_to_sql is failing because of that.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

Copy link
Contributor

@o-nikolas o-nikolas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you run the system test locally to ensure it passes with this new code change?

@vincbeck
Copy link
Contributor Author

Have you run the system test locally to ensure it passes with this new code change?

Yes sir!

@vincbeck vincbeck merged commit ed99893 into apache:main Apr 12, 2024
41 checks passed
@vincbeck vincbeck deleted the vincbeck/fix_sql branch April 12, 2024 21:39
@dabla
Copy link
Contributor

dabla commented Apr 13, 2024

As promised I created a PR which also checks the insert_rows method when an iterator is passed as rows parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants