You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/09/11 02:51:48 UTC
[GitHub] [airflow] subkanthi opened a new pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
subkanthi opened a new pull request #18161:
URL: https://github.com/apache/airflow/pull/18161
Added logic to query if a connection exists before trying to create one in the duplicate connection flow.
closes: #18050
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r724901913
##########
File path: airflow/www/views.py
##########
@@ -3414,34 +3414,56 @@ def action_mulduplicate(self, connections, session=None):
for selected_conn in connections:
new_conn_id = selected_conn.conn_id
match = re.search(r"_copy(\d+)$", selected_conn.conn_id)
+
+ base_conn_id = selected_conn.conn_id
if match:
- conn_id_prefix = selected_conn.conn_id[: match.start()]
- new_conn_id = f"{conn_id_prefix}_copy{int(match.group(1)) + 1}"
- else:
- new_conn_id += '_copy1'
-
- dup_conn = Connection(
- new_conn_id,
- selected_conn.conn_type,
- selected_conn.description,
- selected_conn.host,
- selected_conn.login,
- selected_conn.password,
- selected_conn.schema,
- selected_conn.port,
- selected_conn.extra,
- )
+ base_conn_id = base_conn_id.split('_copy')[0]
+
+ potential_connection_ids = [f"{base_conn_id}_copy{i}" for i in range(1, 11)]
+
+ query = session.query(Connection.conn_id).filter(Connection.conn_id.in_(potential_connection_ids))
+
+ found_conn_id_set = {conn_id for conn_id, in query}
+
+ possible_conn_ids = []
+ for connection_id in potential_connection_ids:
+ if connection_id not in found_conn_id_set:
+ possible_conn_ids.append(connection_id)
+ possible_conn_id_iter = iter(possible_conn_ids)
Review comment:
```suggestion
possible_conn_ids = (
connection_id
for connection_id in potential_connection_ids
if connection_id not in found_conn_id_set
)
```
This would calculate the possible connection IDs lazily, i.e. if `found_conn_id_set` is empty, only one ID would be iterated instead of generating the whole list.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r720776579
##########
File path: airflow/www/views.py
##########
@@ -3409,39 +3410,68 @@ def action_muldelete(self, items):
(permissions.ACTION_CAN_READ, permissions.RESOURCE_CONNECTION),
]
)
- def action_mulduplicate(self, connections, session=None):
+
+def action_mulduplicate(self, connections, session=None):
Review comment:
Unintended?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r724998248
##########
File path: tests/www/views/test_views_connection.py
##########
@@ -148,3 +148,43 @@ def test_duplicate_connection(admin_client):
response = {conn[0] for conn in session.query(Connection.conn_id).all()}
assert resp.status_code == 200
assert expected_result == response
+
+
+def test_duplicate_connection_error(admin_client):
+ """Test Duplicate multiple connection with suffix
+ when there are already 10 copies, no new copy
+ should be created"""
+
+ connections = []
+ connection_ids = []
+
+ for i in range(1, 11):
+ connection_id = f'test_duplicate_postgres_connection_copy{i}'
+ print(f"connection_id {connection_id}")
+ conn1 = Connection(
+ conn_id=connection_id,
+ conn_type='FTP',
+ description='Postgres',
+ host='localhost',
+ schema='airflow',
+ port=3306,
+ )
+ connections.append(conn1)
+ connection_ids.append(conn1.id)
+
+ with create_session() as session:
+ session.query(Connection).delete()
+ session.add_all(connections)
+ session.commit()
Review comment:
Removed session.commit
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#issuecomment-939302987
Wooho!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi closed pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
subkanthi closed pull request #18161:
URL: https://github.com/apache/airflow/pull/18161
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r715747963
##########
File path: airflow/www/views.py
##########
@@ -3350,39 +3350,67 @@ def action_muldelete(self, items):
(permissions.ACTION_CAN_READ, permissions.RESOURCE_CONNECTION),
]
)
+
def action_mulduplicate(self, connections, session=None):
"""Duplicate Multiple connections"""
for selected_conn in connections:
new_conn_id = selected_conn.conn_id
match = re.search(r"_copy(\d+)$", selected_conn.conn_id)
+
+ base_conn_id = selected_conn.conn_id
if match:
- conn_id_prefix = selected_conn.conn_id[: match.start()]
- new_conn_id = f"{conn_id_prefix}_copy{int(match.group(1)) + 1}"
+ base_conn_id = base_conn_id.split('_copy')[0]
+
+ potential_connection_ids = []
+ # Before creating a new connection
+ # Query to see if connection exists.
+ sql_query_string = 'select conn_id from connection where '
+ for i in range(1, 11):
+ connection_id = f'{base_conn_id}_copy{i}'
+ potential_connection_ids.append(connection_id)
+ sql_query_string += f"conn_id='{connection_id}'"
+ if i < 10:
+ sql_query_string += f" or "
+
+ results = session.execute(sql_query_string)
+
+ for result in results:
+ for column, value in result.items():
+ potential_connection_ids.remove(value)
+
+ if len(potential_connection_ids) == 0:
+ # No connections available
+ flash(
+ f"Connection {new_conn_id} can't be added because it already exists, "
+ f"Please rename the existing connections",
+ "warning",
+ )
Review comment:
Hi @uranusjr , Thanks for suggesting this. I have a problem with the next syntax, Is it possible to have a for loop inside next. I thought stopIteration is only called when next is called iteratively. It will be cool to learn.
It will be great if u can suggest the correct syntax for this.
`try:
new_conn_id = next(
connection_id
for connection_id in potential_connection_ids
if connection_id not in {connection_id for connection_id, in query}
)`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r724638059
##########
File path: tests/www/views/test_views_connection.py
##########
@@ -148,3 +148,43 @@ def test_duplicate_connection(admin_client):
response = {conn[0] for conn in session.query(Connection.conn_id).all()}
assert resp.status_code == 200
assert expected_result == response
+
+
+def test_duplicate_connection_error(admin_client):
+ """Test Duplicate multiple connection with suffix
+ when there are already 10 copies, no new copy
+ should be created"""
+
+ connections = []
+ connection_ids = []
+
+ for i in range(1, 11):
+ connection_id = f'test_duplicate_postgres_connection_copy{i}'
+ print(f"connection_id {connection_id}")
+ conn1 = Connection(
+ conn_id=connection_id,
+ conn_type='FTP',
+ description='Postgres',
+ host='localhost',
+ schema='airflow',
+ port=3306,
+ )
+ connections.append(conn1)
+ connection_ids.append(conn1.id)
+
+ with create_session() as session:
+ session.query(Connection).delete()
+ session.add_all(connections)
+ session.commit()
+
+ data = {"action": "mulduplicate", "rowid": [conn1.id]}
+ resp = admin_client.post('/connection/action_post', data=data, follow_redirects=True)
+
+ expected_result = set()
+
+ for i in range(1, 11):
+ expected_result.add(f'test_duplicate_postgres_connection_copy{i}')
Review comment:
```suggestion
expected_result = {f'test_duplicate_postgres_connection_copy{i}' for i in range(1, 11)}
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
subkanthi commented on pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#issuecomment-934652039
> LGTM - but how about adding a test for that one ?
Thanks , sure will do.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
subkanthi commented on pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#issuecomment-934892074
> OK. Ping us when there are tests :)
There is a test in test_views_connection.py that tests the duplication logic, I can add one more to test the logic of erroring out when 10 connections already exists.
`def test_duplicate_connection(admin_client):
"""Test Duplicate multiple connection with suffix"""
conn1 = Connection(
conn_id='test_duplicate_gcp_connection',
conn_type='Google Cloud',
description='Google Cloud Connection',
)
conn2 = Connection(
conn_id='test_duplicate_mysql_connection',
conn_type='FTP',
description='MongoDB2',
host='localhost',
schema='airflow',
port=3306,
)
conn3 = Connection(
conn_id='test_duplicate_postgres_connection_copy1',
conn_type='FTP',
description='Postgres',
host='localhost',
schema='airflow',
port=3306,
)
with create_session() as session:
session.query(Connection).delete()
session.add_all([conn1, conn2, conn3])
session.commit()
data = {"action": "mulduplicate", "rowid": [conn1.id, conn3.id]}
resp = admin_client.post('/connection/action_post', data=data, follow_redirects=True)
expected_result = {
'test_duplicate_gcp_connection',
'test_duplicate_gcp_connection_copy1',
'test_duplicate_mysql_connection',
'test_duplicate_postgres_connection_copy1',
'test_duplicate_postgres_connection_copy2',
}
response = {conn[0] for conn in session.query(Connection.conn_id).all()}
assert resp.status_code == 200
assert expected_result == response`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#issuecomment-934879885
OK. Ping us when there are tests :)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r720681535
##########
File path: airflow/www/views.py
##########
@@ -3350,39 +3350,67 @@ def action_muldelete(self, items):
(permissions.ACTION_CAN_READ, permissions.RESOURCE_CONNECTION),
]
)
+
def action_mulduplicate(self, connections, session=None):
"""Duplicate Multiple connections"""
for selected_conn in connections:
new_conn_id = selected_conn.conn_id
match = re.search(r"_copy(\d+)$", selected_conn.conn_id)
+
+ base_conn_id = selected_conn.conn_id
if match:
- conn_id_prefix = selected_conn.conn_id[: match.start()]
- new_conn_id = f"{conn_id_prefix}_copy{int(match.group(1)) + 1}"
+ base_conn_id = base_conn_id.split('_copy')[0]
+
+ potential_connection_ids = []
+ # Before creating a new connection
+ # Query to see if connection exists.
+ sql_query_string = 'select conn_id from connection where '
+ for i in range(1, 11):
+ connection_id = f'{base_conn_id}_copy{i}'
+ potential_connection_ids.append(connection_id)
+ sql_query_string += f"conn_id='{connection_id}'"
+ if i < 10:
+ sql_query_string += f" or "
+
+ results = session.execute(sql_query_string)
+
+ for result in results:
+ for column, value in result.items():
+ potential_connection_ids.remove(value)
+
+ if len(potential_connection_ids) == 0:
+ # No connections available
+ flash(
+ f"Connection {new_conn_id} can't be added because it already exists, "
+ f"Please rename the existing connections",
+ "warning",
+ )
Review comment:
Thanks, made the change, I now understand the next is really just to get the first element
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r715157470
##########
File path: airflow/www/views.py
##########
@@ -3350,39 +3350,67 @@ def action_muldelete(self, items):
(permissions.ACTION_CAN_READ, permissions.RESOURCE_CONNECTION),
]
)
+
def action_mulduplicate(self, connections, session=None):
"""Duplicate Multiple connections"""
for selected_conn in connections:
new_conn_id = selected_conn.conn_id
match = re.search(r"_copy(\d+)$", selected_conn.conn_id)
+
+ base_conn_id = selected_conn.conn_id
if match:
- conn_id_prefix = selected_conn.conn_id[: match.start()]
- new_conn_id = f"{conn_id_prefix}_copy{int(match.group(1)) + 1}"
+ base_conn_id = base_conn_id.split('_copy')[0]
+
+ potential_connection_ids = []
+ # Before creating a new connection
+ # Query to see if connection exists.
+ sql_query_string = 'select conn_id from connection where '
+ for i in range(1, 11):
+ connection_id = f'{base_conn_id}_copy{i}'
+ potential_connection_ids.append(connection_id)
+ sql_query_string += f"conn_id='{connection_id}'"
+ if i < 10:
+ sql_query_string += f" or "
+
+ results = session.execute(sql_query_string)
+
+ for result in results:
+ for column, value in result.items():
+ potential_connection_ids.remove(value)
Review comment:
```python
potential_connection_ids = {f"{base_conn_id}_copy{i}" for i in range(1, 11)}
query = (
session.query(Connection.connection_id)
.filter(Connection.connection_id.in_(potential_connection_ids)
)
potential_connection_ids.symmetric_difference_update(connection_id for connection_id, in query)
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r715157996
##########
File path: airflow/www/views.py
##########
@@ -3350,39 +3350,67 @@ def action_muldelete(self, items):
(permissions.ACTION_CAN_READ, permissions.RESOURCE_CONNECTION),
]
)
+
def action_mulduplicate(self, connections, session=None):
"""Duplicate Multiple connections"""
for selected_conn in connections:
new_conn_id = selected_conn.conn_id
match = re.search(r"_copy(\d+)$", selected_conn.conn_id)
+
+ base_conn_id = selected_conn.conn_id
if match:
- conn_id_prefix = selected_conn.conn_id[: match.start()]
- new_conn_id = f"{conn_id_prefix}_copy{int(match.group(1)) + 1}"
+ base_conn_id = base_conn_id.split('_copy')[0]
Review comment:
```suggestion
base_conn_id = base_conn_id.split("_copy", 1)[0]
```
Slightly potimised.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #18161:
URL: https://github.com/apache/airflow/pull/18161
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r724901913
##########
File path: airflow/www/views.py
##########
@@ -3414,34 +3414,56 @@ def action_mulduplicate(self, connections, session=None):
for selected_conn in connections:
new_conn_id = selected_conn.conn_id
match = re.search(r"_copy(\d+)$", selected_conn.conn_id)
+
+ base_conn_id = selected_conn.conn_id
if match:
- conn_id_prefix = selected_conn.conn_id[: match.start()]
- new_conn_id = f"{conn_id_prefix}_copy{int(match.group(1)) + 1}"
- else:
- new_conn_id += '_copy1'
-
- dup_conn = Connection(
- new_conn_id,
- selected_conn.conn_type,
- selected_conn.description,
- selected_conn.host,
- selected_conn.login,
- selected_conn.password,
- selected_conn.schema,
- selected_conn.port,
- selected_conn.extra,
- )
+ base_conn_id = base_conn_id.split('_copy')[0]
+
+ potential_connection_ids = [f"{base_conn_id}_copy{i}" for i in range(1, 11)]
+
+ query = session.query(Connection.conn_id).filter(Connection.conn_id.in_(potential_connection_ids))
+
+ found_conn_id_set = {conn_id for conn_id, in query}
+
+ possible_conn_ids = []
+ for connection_id in potential_connection_ids:
+ if connection_id not in found_conn_id_set:
+ possible_conn_ids.append(connection_id)
+ possible_conn_id_iter = iter(possible_conn_ids)
Review comment:
```suggestion
possible_conn_id_iter = (
connection_id
for connection_id in potential_connection_ids
if connection_id not in found_conn_id_set
)
```
This would calculate the possible connection IDs lazily, i.e. if `found_conn_id_set` is empty, only one ID would be iterated instead of generating the whole list.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r715747963
##########
File path: airflow/www/views.py
##########
@@ -3350,39 +3350,67 @@ def action_muldelete(self, items):
(permissions.ACTION_CAN_READ, permissions.RESOURCE_CONNECTION),
]
)
+
def action_mulduplicate(self, connections, session=None):
"""Duplicate Multiple connections"""
for selected_conn in connections:
new_conn_id = selected_conn.conn_id
match = re.search(r"_copy(\d+)$", selected_conn.conn_id)
+
+ base_conn_id = selected_conn.conn_id
if match:
- conn_id_prefix = selected_conn.conn_id[: match.start()]
- new_conn_id = f"{conn_id_prefix}_copy{int(match.group(1)) + 1}"
+ base_conn_id = base_conn_id.split('_copy')[0]
+
+ potential_connection_ids = []
+ # Before creating a new connection
+ # Query to see if connection exists.
+ sql_query_string = 'select conn_id from connection where '
+ for i in range(1, 11):
+ connection_id = f'{base_conn_id}_copy{i}'
+ potential_connection_ids.append(connection_id)
+ sql_query_string += f"conn_id='{connection_id}'"
+ if i < 10:
+ sql_query_string += f" or "
+
+ results = session.execute(sql_query_string)
+
+ for result in results:
+ for column, value in result.items():
+ potential_connection_ids.remove(value)
+
+ if len(potential_connection_ids) == 0:
+ # No connections available
+ flash(
+ f"Connection {new_conn_id} can't be added because it already exists, "
+ f"Please rename the existing connections",
+ "warning",
+ )
Review comment:
Hi @uranusjr , Thanks for suggesting this. I have a problem with the next syntax, Is it possible to have a for loop inside next.
It will be great if u can suggest the correct syntax for this.
`try:
new_conn_id = next(
connection_id
for connection_id in potential_connection_ids
if connection_id not in {connection_id for connection_id, in query}
)`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r706588381
##########
File path: airflow/www/views.py
##########
@@ -3320,28 +3320,38 @@ def action_mulduplicate(self, connections, session=None):
else:
new_conn_id += '_copy1'
- dup_conn = Connection(
- new_conn_id,
- selected_conn.conn_type,
- selected_conn.description,
- selected_conn.host,
- selected_conn.login,
- selected_conn.password,
- selected_conn.schema,
- selected_conn.port,
- selected_conn.extra,
- )
-
- try:
- session.add(dup_conn)
- session.commit()
- flash(f"Connection {new_conn_id} added successfully.", "success")
- except IntegrityError:
+ # Before creating a new connection
+ # Query to see if connection exists.
+ connection = session.query(Connection).filter(Connection.conn_id == new_conn_id).one_or_none()
+ if connection is not None:
flash(
- f"Connection {new_conn_id} can't be added. Integrity error, probably unique constraint.",
+ f"Connection {new_conn_id} can't be added because it lready exists",
Review comment:
How about adding a small loop and trying to add further _1 , _2 , _3 ....... suffix up to, say, 10 times ?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r715157470
##########
File path: airflow/www/views.py
##########
@@ -3350,39 +3350,67 @@ def action_muldelete(self, items):
(permissions.ACTION_CAN_READ, permissions.RESOURCE_CONNECTION),
]
)
+
def action_mulduplicate(self, connections, session=None):
"""Duplicate Multiple connections"""
for selected_conn in connections:
new_conn_id = selected_conn.conn_id
match = re.search(r"_copy(\d+)$", selected_conn.conn_id)
+
+ base_conn_id = selected_conn.conn_id
if match:
- conn_id_prefix = selected_conn.conn_id[: match.start()]
- new_conn_id = f"{conn_id_prefix}_copy{int(match.group(1)) + 1}"
+ base_conn_id = base_conn_id.split('_copy')[0]
+
+ potential_connection_ids = []
+ # Before creating a new connection
+ # Query to see if connection exists.
+ sql_query_string = 'select conn_id from connection where '
+ for i in range(1, 11):
+ connection_id = f'{base_conn_id}_copy{i}'
+ potential_connection_ids.append(connection_id)
+ sql_query_string += f"conn_id='{connection_id}'"
+ if i < 10:
+ sql_query_string += f" or "
+
+ results = session.execute(sql_query_string)
+
+ for result in results:
+ for column, value in result.items():
+ potential_connection_ids.remove(value)
Review comment:
```python
potential_connection_ids = [f"{base_conn_id}_copy{i}" for i in range(1, 11)]
query = (
session.query(Connection.connection_id)
.filter(Connection.connection_id.in_(potential_connection_ids)
)
potential_connection_ids = [
connection_id
for connection_id in potential_connection_ids
if connection_id not in {connection_id for connection_id, in query}
]
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r715159778
##########
File path: airflow/www/views.py
##########
@@ -3350,39 +3350,67 @@ def action_muldelete(self, items):
(permissions.ACTION_CAN_READ, permissions.RESOURCE_CONNECTION),
]
)
+
def action_mulduplicate(self, connections, session=None):
"""Duplicate Multiple connections"""
for selected_conn in connections:
new_conn_id = selected_conn.conn_id
match = re.search(r"_copy(\d+)$", selected_conn.conn_id)
+
+ base_conn_id = selected_conn.conn_id
if match:
- conn_id_prefix = selected_conn.conn_id[: match.start()]
- new_conn_id = f"{conn_id_prefix}_copy{int(match.group(1)) + 1}"
+ base_conn_id = base_conn_id.split('_copy')[0]
+
+ potential_connection_ids = []
+ # Before creating a new connection
+ # Query to see if connection exists.
+ sql_query_string = 'select conn_id from connection where '
+ for i in range(1, 11):
+ connection_id = f'{base_conn_id}_copy{i}'
+ potential_connection_ids.append(connection_id)
+ sql_query_string += f"conn_id='{connection_id}'"
+ if i < 10:
+ sql_query_string += f" or "
+
+ results = session.execute(sql_query_string)
+
+ for result in results:
+ for column, value in result.items():
+ potential_connection_ids.remove(value)
+
+ if len(potential_connection_ids) == 0:
+ # No connections available
+ flash(
+ f"Connection {new_conn_id} can't be added because it already exists, "
+ f"Please rename the existing connections",
+ "warning",
+ )
Review comment:
```suggestion
if not potential_connection_ids:
# No connections available
flash(
f"Connection {new_conn_id} can't be added because it already exists, "
f"Please rename the existing connections",
"warning",
)
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r724637820
##########
File path: tests/www/views/test_views_connection.py
##########
@@ -148,3 +148,43 @@ def test_duplicate_connection(admin_client):
response = {conn[0] for conn in session.query(Connection.conn_id).all()}
assert resp.status_code == 200
assert expected_result == response
+
+
+def test_duplicate_connection_error(admin_client):
+ """Test Duplicate multiple connection with suffix
+ when there are already 10 copies, no new copy
+ should be created"""
+
+ connections = []
+ connection_ids = []
+
+ for i in range(1, 11):
+ connection_id = f'test_duplicate_postgres_connection_copy{i}'
+ print(f"connection_id {connection_id}")
+ conn1 = Connection(
+ conn_id=connection_id,
+ conn_type='FTP',
+ description='Postgres',
+ host='localhost',
+ schema='airflow',
+ port=3306,
+ )
+ connections.append(conn1)
+ connection_ids.append(conn1.id)
+
+ with create_session() as session:
+ session.query(Connection).delete()
+ session.add_all(connections)
+ session.commit()
Review comment:
Don't need this, `create_session` automatically commits on exit.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r725024609
##########
File path: airflow/www/views.py
##########
@@ -3414,34 +3414,60 @@ def action_mulduplicate(self, connections, session=None):
for selected_conn in connections:
new_conn_id = selected_conn.conn_id
match = re.search(r"_copy(\d+)$", selected_conn.conn_id)
+
+ base_conn_id = selected_conn.conn_id
if match:
- conn_id_prefix = selected_conn.conn_id[: match.start()]
- new_conn_id = f"{conn_id_prefix}_copy{int(match.group(1)) + 1}"
- else:
- new_conn_id += '_copy1'
-
- dup_conn = Connection(
- new_conn_id,
- selected_conn.conn_type,
- selected_conn.description,
- selected_conn.host,
- selected_conn.login,
- selected_conn.password,
- selected_conn.schema,
- selected_conn.port,
- selected_conn.extra,
- )
+ base_conn_id = base_conn_id.split('_copy')[0]
+
+ potential_connection_ids = [f"{base_conn_id}_copy{i}" for i in range(1, 11)]
+
+ query = session.query(Connection.conn_id).filter(Connection.conn_id.in_(potential_connection_ids))
+
+ found_conn_id_set = {conn_id for conn_id, in query}
+
+ possible_conn_ids = []
+ for connection_id in potential_connection_ids:
+ if connection_id not in found_conn_id_set:
+ possible_conn_ids.append(connection_id)
Review comment:
This block is no longer used?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r725416651
##########
File path: airflow/www/views.py
##########
@@ -3414,34 +3414,60 @@ def action_mulduplicate(self, connections, session=None):
for selected_conn in connections:
new_conn_id = selected_conn.conn_id
match = re.search(r"_copy(\d+)$", selected_conn.conn_id)
+
+ base_conn_id = selected_conn.conn_id
if match:
- conn_id_prefix = selected_conn.conn_id[: match.start()]
- new_conn_id = f"{conn_id_prefix}_copy{int(match.group(1)) + 1}"
- else:
- new_conn_id += '_copy1'
-
- dup_conn = Connection(
- new_conn_id,
- selected_conn.conn_type,
- selected_conn.description,
- selected_conn.host,
- selected_conn.login,
- selected_conn.password,
- selected_conn.schema,
- selected_conn.port,
- selected_conn.extra,
- )
+ base_conn_id = base_conn_id.split('_copy')[0]
+
+ potential_connection_ids = [f"{base_conn_id}_copy{i}" for i in range(1, 11)]
+
+ query = session.query(Connection.conn_id).filter(Connection.conn_id.in_(potential_connection_ids))
+
+ found_conn_id_set = {conn_id for conn_id, in query}
+
+ possible_conn_ids = []
+ for connection_id in potential_connection_ids:
+ if connection_id not in found_conn_id_set:
+ possible_conn_ids.append(connection_id)
Review comment:
Removed, thanks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r724638456
##########
File path: tests/www/views/test_views_connection.py
##########
@@ -148,3 +148,43 @@ def test_duplicate_connection(admin_client):
response = {conn[0] for conn in session.query(Connection.conn_id).all()}
assert resp.status_code == 200
assert expected_result == response
+
+
+def test_duplicate_connection_error(admin_client):
+ """Test Duplicate multiple connection with suffix
+ when there are already 10 copies, no new copy
+ should be created"""
+
+ connections = []
+ connection_ids = []
+
+ for i in range(1, 11):
+ connection_id = f'test_duplicate_postgres_connection_copy{i}'
+ print(f"connection_id {connection_id}")
+ conn1 = Connection(
+ conn_id=connection_id,
+ conn_type='FTP',
+ description='Postgres',
+ host='localhost',
+ schema='airflow',
+ port=3306,
+ )
+ connections.append(conn1)
+ connection_ids.append(conn1.id)
Review comment:
```suggestion
connection_ids = [f'test_duplicate_postgres_connection_copy{i}' for i in range(1, 11)]
connections = [
Connection(
conn_id=connection_id,
conn_type='FTP',
description='Postgres',
host='localhost',
schema='airflow',
port=3306,
)
for connection_id in connection_ids
]
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#issuecomment-934927282
> There is a test in test_views_connection.py that tests the duplication logic, I can add one more to test the logic of erroring out when 10 connections already exists.
sounds good :)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r715157996
##########
File path: airflow/www/views.py
##########
@@ -3350,39 +3350,67 @@ def action_muldelete(self, items):
(permissions.ACTION_CAN_READ, permissions.RESOURCE_CONNECTION),
]
)
+
def action_mulduplicate(self, connections, session=None):
"""Duplicate Multiple connections"""
for selected_conn in connections:
new_conn_id = selected_conn.conn_id
match = re.search(r"_copy(\d+)$", selected_conn.conn_id)
+
+ base_conn_id = selected_conn.conn_id
if match:
- conn_id_prefix = selected_conn.conn_id[: match.start()]
- new_conn_id = f"{conn_id_prefix}_copy{int(match.group(1)) + 1}"
+ base_conn_id = base_conn_id.split('_copy')[0]
Review comment:
```suggestion
base_conn_id = selected_conn.conn_id[: match.start()]
```
Avoid parsing the ID twice.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r716148625
##########
File path: airflow/www/views.py
##########
@@ -3350,39 +3350,67 @@ def action_muldelete(self, items):
(permissions.ACTION_CAN_READ, permissions.RESOURCE_CONNECTION),
]
)
+
def action_mulduplicate(self, connections, session=None):
"""Duplicate Multiple connections"""
for selected_conn in connections:
new_conn_id = selected_conn.conn_id
match = re.search(r"_copy(\d+)$", selected_conn.conn_id)
+
+ base_conn_id = selected_conn.conn_id
if match:
- conn_id_prefix = selected_conn.conn_id[: match.start()]
- new_conn_id = f"{conn_id_prefix}_copy{int(match.group(1)) + 1}"
+ base_conn_id = base_conn_id.split('_copy')[0]
+
+ potential_connection_ids = []
+ # Before creating a new connection
+ # Query to see if connection exists.
+ sql_query_string = 'select conn_id from connection where '
+ for i in range(1, 11):
+ connection_id = f'{base_conn_id}_copy{i}'
+ potential_connection_ids.append(connection_id)
+ sql_query_string += f"conn_id='{connection_id}'"
+ if i < 10:
+ sql_query_string += f" or "
+
+ results = session.execute(sql_query_string)
+
+ for result in results:
+ for column, value in result.items():
+ potential_connection_ids.remove(value)
+
+ if len(potential_connection_ids) == 0:
+ # No connections available
+ flash(
+ f"Connection {new_conn_id} can't be added because it already exists, "
+ f"Please rename the existing connections",
+ "warning",
+ )
Review comment:
This is the correct syntax. `next()` accepts an iterator, and either returns the next element in the iterator, or raises `StopIteration` if there is no next element (i.e. the iterator has been exhausted).
The thing inside `next()` is called an iterator comprehension. The entire function call is roughly equivalent to
```python
found_conn_id_set = {connection_id for connection_id, in query}
possible_conn_ids = []
for connection_id in potential_connection_ids:
if connection_id not in found_conn_id_set:
possible_conn_ids.append(connection_id)
possible_conn_id_iter = iter(possible_conn_ids)
try:
new_conn_id = next(possible_conn_id_iter)
except StopIteration:
flash(...)
```
But with better performance.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r715163399
##########
File path: airflow/www/views.py
##########
@@ -3350,39 +3350,67 @@ def action_muldelete(self, items):
(permissions.ACTION_CAN_READ, permissions.RESOURCE_CONNECTION),
]
)
+
def action_mulduplicate(self, connections, session=None):
"""Duplicate Multiple connections"""
for selected_conn in connections:
new_conn_id = selected_conn.conn_id
match = re.search(r"_copy(\d+)$", selected_conn.conn_id)
+
+ base_conn_id = selected_conn.conn_id
if match:
- conn_id_prefix = selected_conn.conn_id[: match.start()]
- new_conn_id = f"{conn_id_prefix}_copy{int(match.group(1)) + 1}"
+ base_conn_id = base_conn_id.split('_copy')[0]
+
+ potential_connection_ids = []
+ # Before creating a new connection
+ # Query to see if connection exists.
+ sql_query_string = 'select conn_id from connection where '
+ for i in range(1, 11):
+ connection_id = f'{base_conn_id}_copy{i}'
+ potential_connection_ids.append(connection_id)
+ sql_query_string += f"conn_id='{connection_id}'"
+ if i < 10:
+ sql_query_string += f" or "
+
+ results = session.execute(sql_query_string)
+
+ for result in results:
+ for column, value in result.items():
+ potential_connection_ids.remove(value)
+
+ if len(potential_connection_ids) == 0:
+ # No connections available
+ flash(
+ f"Connection {new_conn_id} can't be added because it already exists, "
+ f"Please rename the existing connections",
+ "warning",
+ )
Review comment:
```python
potential_connection_ids = [f"{base_conn_id}_copy{i}" for i in range(1, 11)]
query = (
session.query(Connection.connection_id)
.filter(Connection.connection_id.in_(potential_connection_ids)
)
try:
new_conn_id = next(
connection_id
for connection_id in potential_connection_ids
if connection_id not in {connection_id for connection_id, in query}
)
except StopIteration:
flash(...)
else:
... # Create the connection.
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi closed pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
subkanthi closed pull request #18161:
URL: https://github.com/apache/airflow/pull/18161
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r720816770
##########
File path: airflow/www/views.py
##########
@@ -3409,39 +3410,68 @@ def action_muldelete(self, items):
(permissions.ACTION_CAN_READ, permissions.RESOURCE_CONNECTION),
]
)
- def action_mulduplicate(self, connections, session=None):
+
+def action_mulduplicate(self, connections, session=None):
Review comment:
yes definitely.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r715747963
##########
File path: airflow/www/views.py
##########
@@ -3350,39 +3350,67 @@ def action_muldelete(self, items):
(permissions.ACTION_CAN_READ, permissions.RESOURCE_CONNECTION),
]
)
+
def action_mulduplicate(self, connections, session=None):
"""Duplicate Multiple connections"""
for selected_conn in connections:
new_conn_id = selected_conn.conn_id
match = re.search(r"_copy(\d+)$", selected_conn.conn_id)
+
+ base_conn_id = selected_conn.conn_id
if match:
- conn_id_prefix = selected_conn.conn_id[: match.start()]
- new_conn_id = f"{conn_id_prefix}_copy{int(match.group(1)) + 1}"
+ base_conn_id = base_conn_id.split('_copy')[0]
+
+ potential_connection_ids = []
+ # Before creating a new connection
+ # Query to see if connection exists.
+ sql_query_string = 'select conn_id from connection where '
+ for i in range(1, 11):
+ connection_id = f'{base_conn_id}_copy{i}'
+ potential_connection_ids.append(connection_id)
+ sql_query_string += f"conn_id='{connection_id}'"
+ if i < 10:
+ sql_query_string += f" or "
+
+ results = session.execute(sql_query_string)
+
+ for result in results:
+ for column, value in result.items():
+ potential_connection_ids.remove(value)
+
+ if len(potential_connection_ids) == 0:
+ # No connections available
+ flash(
+ f"Connection {new_conn_id} can't be added because it already exists, "
+ f"Please rename the existing connections",
+ "warning",
+ )
Review comment:
Hi @uranusjr , Thanks for suggesting this. I have a problem with the code(especially the lines inside next()), Is it possible to have a for loop inside next. I thought stopIteration is only called when next is called iteratively. It will be cool to learn.
It will be great if u can suggest the correct syntax for this.
`try:
new_conn_id = next(
connection_id
for connection_id in potential_connection_ids
if connection_id not in {connection_id for connection_id, in query}
)`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#issuecomment-934638942
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.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r725008485
##########
File path: tests/www/views/test_views_connection.py
##########
@@ -148,3 +148,43 @@ def test_duplicate_connection(admin_client):
response = {conn[0] for conn in session.query(Connection.conn_id).all()}
assert resp.status_code == 200
assert expected_result == response
+
+
+def test_duplicate_connection_error(admin_client):
+ """Test Duplicate multiple connection with suffix
+ when there are already 10 copies, no new copy
+ should be created"""
+
+ connections = []
+ connection_ids = []
+
+ for i in range(1, 11):
+ connection_id = f'test_duplicate_postgres_connection_copy{i}'
+ print(f"connection_id {connection_id}")
+ conn1 = Connection(
+ conn_id=connection_id,
+ conn_type='FTP',
+ description='Postgres',
+ host='localhost',
+ schema='airflow',
+ port=3306,
+ )
+ connections.append(conn1)
+ connection_ids.append(conn1.id)
+
+ with create_session() as session:
+ session.query(Connection).delete()
+ session.add_all(connections)
+ session.commit()
Review comment:
Done
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r706588042
##########
File path: airflow/www/views.py
##########
@@ -3320,28 +3320,38 @@ def action_mulduplicate(self, connections, session=None):
else:
new_conn_id += '_copy1'
- dup_conn = Connection(
- new_conn_id,
- selected_conn.conn_type,
- selected_conn.description,
- selected_conn.host,
- selected_conn.login,
- selected_conn.password,
- selected_conn.schema,
- selected_conn.port,
- selected_conn.extra,
- )
-
- try:
- session.add(dup_conn)
- session.commit()
- flash(f"Connection {new_conn_id} added successfully.", "success")
- except IntegrityError:
+ # Before creating a new connection
+ # Query to see if connection exists.
+ connection = session.query(Connection).filter(Connection.conn_id == new_conn_id).one_or_none()
+ if connection is not None:
flash(
- f"Connection {new_conn_id} can't be added. Integrity error, probably unique constraint.",
+ f"Connection {new_conn_id} can't be added because it lready exists",
Review comment:
```suggestion
f"Connection {new_conn_id} can't be added because it already exists",
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] subkanthi commented on a change in pull request #18161: Duplicate Connection: Added logic to query if a connection id exists before creating one
Posted by GitBox <gi...@apache.org>.
subkanthi commented on a change in pull request #18161:
URL: https://github.com/apache/airflow/pull/18161#discussion_r715747963
##########
File path: airflow/www/views.py
##########
@@ -3350,39 +3350,67 @@ def action_muldelete(self, items):
(permissions.ACTION_CAN_READ, permissions.RESOURCE_CONNECTION),
]
)
+
def action_mulduplicate(self, connections, session=None):
"""Duplicate Multiple connections"""
for selected_conn in connections:
new_conn_id = selected_conn.conn_id
match = re.search(r"_copy(\d+)$", selected_conn.conn_id)
+
+ base_conn_id = selected_conn.conn_id
if match:
- conn_id_prefix = selected_conn.conn_id[: match.start()]
- new_conn_id = f"{conn_id_prefix}_copy{int(match.group(1)) + 1}"
+ base_conn_id = base_conn_id.split('_copy')[0]
+
+ potential_connection_ids = []
+ # Before creating a new connection
+ # Query to see if connection exists.
+ sql_query_string = 'select conn_id from connection where '
+ for i in range(1, 11):
+ connection_id = f'{base_conn_id}_copy{i}'
+ potential_connection_ids.append(connection_id)
+ sql_query_string += f"conn_id='{connection_id}'"
+ if i < 10:
+ sql_query_string += f" or "
+
+ results = session.execute(sql_query_string)
+
+ for result in results:
+ for column, value in result.items():
+ potential_connection_ids.remove(value)
+
+ if len(potential_connection_ids) == 0:
+ # No connections available
+ flash(
+ f"Connection {new_conn_id} can't be added because it already exists, "
+ f"Please rename the existing connections",
+ "warning",
+ )
Review comment:
Hi @uranusjr , Thanks for suggesting this. I have a problem with the code(especially the lines inside next()), Is it possible to have a for loop inside next. I thought stopIteration is only called when next is called iteratively. It will be cool to learn.
It will be great if u can suggest the correct syntax for this.
```python
try:
new_conn_id = next(
connection_id
for connection_id in potential_connection_ids
if connection_id not in {connection_id for connection_id, in query}
)
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org