Make connection login and password TEXT

[Sighery]

This commit will change the connection login and password fields to be of type TEXT, so it won't have a length limitation anymore.

Closes #32657

[boring-cyborg]

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/main/CONTRIBUTING.rst)
Here are some useful points:

• Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits 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 Consider adding an example DAG that shows how users should use it.
• Consider using Breeze environment 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 for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
• Be sure to read the Airflow Coding style.
  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

[hussein-awala]

It appears like MySQL doesn't like your change:

sqlalchemy.exc.OperationalError: (MySQLdb.OperationalError) (1118, 'Row size too large. The maximum row size for the used table type, not counting BLOBs, is 65535. This includes storage overhead, check the manual. You have to change some columns to TEXT or BLOBs')

[Sighery]

It appears like MySQL doesn't like your change:

sqlalchemy.exc.OperationalError: (MySQLdb.OperationalError) (1118, 'Row size too large. The maximum row size for the used table type, not counting BLOBs, is 65535. This includes storage overhead, check the manual. You have to change some columns to TEXT or BLOBs')

Yep. I was talking about it in the related issue. The TL;DR is that it doesn't fail on my migration (128), nor in its downgrade. But rather it fails to downgrade migration 79 after applying that and 128 just fine.

[uranusjr]

Why do we even have a limit anyway? Since we don’t search against the column we can just use Text.

[Sighery]

Why do we even have a limit anyway? Since we don’t search against the column we can just use Text.

For my usecase that'd also work. I don't think there's an index against either login, password, nor extra. Nor any search functionality for those fields. So I think it would make sense to make them all TEXT.

But I'm new to Airflow, as well as the codebase, so figured I'd open an issue first to ask about it, and then I pushed the code to see if current DB engines would support this change (clearly MySQL doesn't for some reason), and get opinions from Airflow committers/devs.

[eladkal]

For my usecase that'd also work. I don't think there's an index against either login, password, nor extra. Nor any search functionality for those fields. So I think it would make sense to make them all TEXT.

can you modify the PR for that?

[Sighery]

can you modify the PR for that?

Sure, sounds good. I'm guessing it will require rebasing and getting it up to date now that 2.7.0 is out

[potiuk]

Yep. Rebase early-rebase often (as I often repeat)

[Sighery]

@eladkal @potiuk Should be up to date now. Apologies it took me so long to get to it, it just so happened that I was on vacation when the decision for switching to TEXT was made.

I might have missed adding something. If so, just let me know 🙌

[Sighery]

Seems what's failing is TestDatabricksExecutionTrigger.test_sleep_between_retries since the test expects the sleep mock to be called once but it was called 6 times. I tried looking at the test code but I can't get a clear view of how this is connected with these changes. If someone more knowledgeable can take a look, or point me on what to look at to determine if this PR indirectly caused that error, would appreciate it.

[boring-cyborg]

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

[hussein-awala]

Congrats on your first commit 🎉