[AIRFLOW-2030] Fix KeyError:i in DbApiHook for insert

[untwal]

Make sure you have checked all steps below.

JIRA

• My PR addresses the following Airflow JIRA issues and references them in the PR title. For example, "[AIRFLOW-2030] My Airflow PR"
  • https://issues.apache.org/jira/browse/AIRFLOW-2030

Description

• No locals() return key i, when there are zero rows in cursor to insert.

Tests

• My PR does not need testing for this extremely good reason: because it's a bug which was handled earlier

Commits

• My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":

  1. Subject is separated from body by a blank line
  2. Subject is limited to 50 characters
  3. Subject does not end with a period
  4. Subject uses the imperative mood ("add", not "adding")
  5. Body wraps at 72 characters
  6. Body explains "what" and "why", not "how"

• Passes git diff upstream/master -u -- "*.py" | flake8 --diff

[codecov-io]

Codecov Report

Merging #2972 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2972      +/-   ##
==========================================
+ Coverage   73.12%   73.13%   +<.01%     
==========================================
  Files         174      174              
  Lines       12318    12319       +1     
==========================================
+ Hits         9008     9009       +1     
  Misses       3310     3310

Impacted Files	Coverage Δ
airflow/hooks/dbapi_hook.py	76.52% <100%> (+0.2%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cbc02da...0416e81. Read the comment docs.

[Fokko]

Can you post the error? I think the i is defined in for i, row in enumerate(rows, 1):, but it isn't in the locals apparently.

[untwal]

for i, row in enumerate(rows, 1):

is not working for line 225:

self.log.info("Done loading. Loaded a total of {i} rows".format(**locals()))

when rows is empty, locals() will not have i but when rows are there then this i will be equal to the numbers of rows

[untwal]

@Fokko please check this one too

[Fokko]

Hi @untwal. Changing the logline to:

self.log.info("Loaded %s into %s rows so far", i, table)

Would also solve the issue and make the code a bit more beautiful. Introducing this i = 0 is a hack in my opinion.

[untwal]

Hi @Fokko, but this variable won't be present because these rows are empty, will throw a NameError at 225 line. I can better write it as

self.log.info("Done loading. Loaded a total of {0} rows".format(locals().get('i', 0)))

this i get define after the for loop starts, will not be defined if rows are empty

[Fokko]

LOL, I was looking at the wrong line. I'm sorry. My suggestion would be:

self.log.info("Done loading. Loaded a total of %s rows", len(rows))

Having this mutable variable that is possibly being overwritten is not so nice.

[untwal]

@Fokko this len() will throw error for other types like if someone is writing it using some cursor like for SomeDBToDBOperators, just checked for my own custom written operator for postgresqlToPostgresqlOperator

[untwal]

@Fokko i here serves as the counter only for the rows so this makes sense to set it to zero at the start, if you suggest something else, I can change it again.

[Fokko]

Fair enough, thanks @untwal !