potiuk commented on a change in pull request #5786: [AIRFLOW-5170]
[AIRFLOW-5256] Consistent licences for python files and related pylint fixes
URL: https://github.com/apache/airflow/pull/5786#discussion_r319969258
##########
File path:
airflow/migrations/versions/33ae817a1ff4_add_kubernetes_resource_checkpointing.py
##########
@@ -44,11 +44,11 @@ def upgrade():
conn = op.get_bind()
# alembic creates an invalid SQL for mssql and mysql dialects
- if conn.dialect.name in ("mysql"):
+ if conn.dialect.name in {"mysql"}:
Review comment:
Yeah. The conditions were actually wrong so I fixed them :).
The "in" operator for strings tests for substring match:
```
For the string and bytes types, x in y is True if and only if x is a
substring of y. An equivalent test is y.find(x) != -1. Empty strings are always
considered to be a substring of any other string, so "" in "abc" will return
True.
```
The original condition `conn.dialect.name in ("mysql")` is equivalent to
`conn.dialect.name in "mysql"` - the parenthesis are superfluous and they
suggest testing `in` against a tuple. I suspect that the original test was
actually with the tuple originally ("mysql", "mssql") and someone split the if
to mysql/mssql later on but did not notice that the tuple was removed along the
way.
Good test in this case would have been `conn.dialect.name in ("mysql",)`
(note the coma), but because it is so easy miss the coma and it's not at all
obvious, I prefer matching against set rather than tuple (there is no way to
make similar mistake in set) - that's why I modified it to use sets in both
cases.
It was working fine before - of course - as substrings tested were not
ambiguous across different dialects but nevertheless, I think it's worth to fix
it (but It would be too much overhead to create a separate issue/commit just to
fix those).
What do you think @feluelle ?
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
