Add means to Duplicate connections from UI

[pateash]

closes: #12401

---

Description

• This change allows users to duplicate connections via UI.
• user can also create duplicates of multiple connections.
• A descriptive flash message will be shown regarding the success or error in any of the connection duplication.

[pateash]

• all the connections will be suffixed with _Copy(NUM) similar to Windows file explorer

[pateash]

It will show a user-friendly error in case of name conflict ( unique integrity)

[jhtimmins]

@ryanahamilton @ashb Bumping y'all on this since you're code owners here.

I think the code looks solid (nice job on the string update logic @pateash). My only nitpick is if _Copy should be uppercased. If connections typically have snakecase titles, it'd be cleaner to go lowercase (_copy). But if no one else minds then I'm fine with it.

[xinbinhuang]

I have the same feeling as @jhtimmins : should it be snakecase _copy instead?

Another thing is: can you also a test case to cover the duplicate logic?
Otherwise, the change looks neat 👍

[ashb]

As others have said, we should have it as _copy not _Copy.

Additionally, it should be copy1 not copy(1) so it can be set via env vars if has to. (You can't put () in env var names, so AIRFLOW_CONN_FOO_COPY(1) would not be possible

[pateash]

As others have said, we should have it as _copy not _Copy.

Additionally, it should be copy1 not copy(1) so it can be set via env vars if has to. (You can't put () in env var names, so AIRFLOW_CONN_FOO_COPY(1) would not be possible

Thanks All for valuable comments,
As per suggestions, I have used _copyN Syntax for duplication,
also test has been added.

[pateash]

@ryanahamilton @ashb
Could you please review and let me know if any more changes required.

[pateash]

As others have said, we should have it as _copy not _Copy.

Additionally, it should be copy1 not copy(1) so it can be set via env vars if has to. (You can't put () in env var names, so AIRFLOW_CONN_FOO_COPY(1) would not be possible

done

[uranusjr]

Nice!

[potiuk]

Can you please rebase and solve conflicts @pateash ?

[pateash]

Can you please rebase and solve conflicts @pateash ?

Thanks @potiuk , done.

[potiuk]

@ashb -> any more comments or can we merge it ?

[jhtimmins]

@ashb can you check whether your requested changes have been addressed?

[github-actions]

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.

[jyotsa09]

There is a scenario where I feel this is not working as expected.

• When I have created a connection with name conn_test_copy1 and then I have tried to duplicate this connection. It actually created a connection with name as conn_test_copy2 instead of conn_test_copy1_copy1.
  

Also in the above scenario if I am trying to duplicate the conn_test_copy1 it's giving me an error of Connection conn_test_copy2 can't be added. Integrity error, probably unique constraint. It's because connection with name conn_test_copy2 is already there but it's looking like we can't duplicate same connection twice.

[pateash]

@jyotsa09
this is expected behaviour, you should not have any connection with _copyN in the end,
it's just a way to create duplicates, after duplication you are supposed to rename the connection.

[uranusjr]

We should detect this and show a better error message. @jyotsa09 Could you file an issue for this?