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 (flake8, 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: [email protected]
  Slack: https://s.apache.org/airflow-slack

The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.

Static checks need fixing

Thanks @potiuk! That's odd that mypy is complaining about this now, and wasn't before 🤔. Will try to fix this.

Thanks @potiuk! That's odd that mypy is complaining about this now, and wasn't before thinking. Will try to fix this.

Mypy does seem to work a bit "unstable'y" - I think I know why and might be we will soon fix it.

It's a bit sad that we need to have this additional job_id variable to keep mypy happy, it might be confusing for future contributors. Do you want me to add some additional comment about this in the code? On the bright side - fewer line changes in this PR.

It's a bit sad that we need to have this additional job_id variable to keep mypy happy, it might be confusing for future contributors. Do you want me to add some additional comment about this in the code? On the bright side - fewer line changes in this PR.

I don't find it wrong. It's not to keep mypy happy, it's to make sure you don't accidentally use None. There is nothing wrong with the way you've done and it needs no explanation

I think in this case mypy gave you hint on how to make your code more redable and less error prone.

Contraray to popular belief peeople should optimise their code for easines of reading and correctness not to "avoid" creating variables. Compileres and interpreters will optimise it away and creating extra, properly named variable is often much better solution than .. adding a comment about it.

I often do that. In this case maybe even name it "new_job_id" and even expilcitly mark it as : str -> that will be the best "explanation" of what it is for and why it's there. And there will be no need for comment.

Comments are overrated on the other hand.

Comments are overrated on the other hand.

Definitely 👍

Could you please clarify if you are ok with the current PR, or do you prefer to s/job_id/new_job_id and add :str to the definition of new_job_id. Either way is ok with me.

Add it - you will see that it's going to be cleaner. And then you can even move self.job_id = new_job_id closer to the end of the function - and separate variabe will suddenly make a lot more sense

Thanks for the guidance @potiuk! 🙇 Ok, so this PR is approved and labeled with okay to merge, tests are green - can I assume it will be merged at your convenience?

Awesome work, congrats on your first merged pull request!

Done :)

Awesome, thank you very much @potiuk 😄