Fix problem with inability to remove fields from Connection form

[MaksYermak]

In current PR I've fixed a problem with inability to remove fields from Connection form. After changing in Connection's form validation, which was made in this PR https://github.com/apache/airflow/pull/23241 , users can't remove value from fields in Connection form. Each time when users try to remove the value and save changes, and then open this Connection's form one more time the value reverts as if nothing changed.

---

^ 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.

[VladaZakharova]

Hi @josh-fell ! Can you please check changes here? Thanks!

[github-actions[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

[potiuk]

I am not sure what exactly problem with if != "" was (because it is slightly not precise - and the exact scemario described in #23241 does not explain it in detail - but I guess if we won't check it, we can accidentally modify fields when they are removed between the connections.

I tried to follow the scenario described in #23241 but I could not see anything wrong with it.

This whole Connection editing form will have to be recreated in Airflow 3 - and I just realized that just few days ago https://github.com/apache/airflow/pull/41144 has been merged that will show the warning in 2.10.1 that in order to remove values you need to recreate the connection - while not perfect - I would hesitate to merge this one until we are sure what was the reason and scenario we wanted to protect (@josh-fell @dstandish - maybe you will be able to dig it out - I know it's 2 years old, but there was a discussion about the if and it apparently made sense then).

[VladaZakharova]

hi @josh-fell @dstandish! Please check the changes here in PR, thanks!

[VladaZakharova]

Hi @potiuk ! Maybe there is someone else who can also review changes here? I think it can take some time for @josh-fell @dstandish to review it...

[potiuk]

@josh-fell @dstandish -> do you recall the scenario this please?

[VladaZakharova]

hi! @josh-fell @dstandish can you please check this one?

[MaksYermak]

Hi @josh-fell @dstandish could you please look on this PR?

[potiuk]

@josh-fell @dstandish -> any comments? This one seems like a good candidate for a bug fix to be backported to 2.10, as long as we somehow figure out if the edge case vaguely explained in the comments is not affected.

[potiuk]

@VladaZakharova @MaksYermak -> I see we have no feedback here, but my question is - how eager you are to get this merged? There are some risks involved if we release it in 2.10.3 - because we want to really avoid relasing 2.10.4 quickly. On the other hand the whole mechanism of connection UI management will be rewritten from scratch in Airflow 3. And there are workarounds for the problem (delete and recreate the connection).

So without having any clue from @josh-fell and @dstandish (they likely don't remember either, though it would be nice to hear back) about the actual problem that introduced this "limitation" - I'd say it's a bit risky to merge that one and backport it for 2.10.3.

Is there an important reason why we want to get this case fixed?

[MaksYermak]

@VladaZakharova @MaksYermak -> I see we have no feedback here, but my question is - how eager you are to get this merged? There are some risks involved if we release it in 2.10.3 - because we want to really avoid relasing 2.10.4 quickly. On the other hand the whole mechanism of connection UI management will be rewritten from scratch in Airflow 3. And there are workarounds for the problem (delete and recreate the connection).

So without having any clue from @josh-fell and @dstandish (they likely don't remember either, though it would be nice to hear back) about the actual problem that introduced this "limitation" - I'd say it's a bit risky to merge that one and backport it for 2.10.3.

Is there an important reason why we want to get this case fixed?

Hello @potiuk In Composer we have a customer issue related to this problem. Also I have one more solution for this problem which is to save current behavior and delete value if the user decides to delete it on UI. Here's the code:

if value != "":
    extra[field_name] = value
elif field_name in extra:
    del extra[field_name]

In my opinion this solution is not risky, because the current behavior is saved. And if we don't have any answer from contributors we can use this solution as a safety one for fixing this problem. WDYT?

[potiuk]

Hard to say - but - can you propose the customer to delete and recreate the. connection

[github-actions[bot]]

Backport successfully created: v2-10-test

Status	Branch	Result
✅	v2-10-test