Copilot commented on code in PR #64498:
URL: https://github.com/apache/airflow/pull/64498#discussion_r3066473214
##########
airflow-core/src/airflow/cli/commands/connection_command.py:
##########
@@ -411,26 +411,38 @@ def _import_helper(file_path: str, overwrite: bool) ->
None:
:param overwrite: Whether to skip or overwrite on collision.
"""
connections_dict = load_connections_dict(file_path)
+ suc_count = fail_count = 0
Review Comment:
`suc_count` is an unclear abbreviation. Rename to something explicit like
`success_count` (and keep `fail_count` as `failure_count` for symmetry) to
improve readability.
##########
airflow-core/src/airflow/cli/commands/connection_command.py:
##########
@@ -411,26 +411,38 @@ def _import_helper(file_path: str, overwrite: bool) ->
None:
:param overwrite: Whether to skip or overwrite on collision.
"""
connections_dict = load_connections_dict(file_path)
+ suc_count = fail_count = 0
with create_session() as session:
for conn_id, conn in connections_dict.items():
try:
helpers.validate_key(conn_id, max_length=200)
except Exception as e:
print(f"Could not import connection. {e}")
Review Comment:
The error messages are inconsistent (one omits `conn_id`, and the other uses
`repr` formatting via `{e!r}` which can be noisy). For a more consistent CLI
UX, include `conn_id` in both cases and standardize formatting (e.g., include
the exception type + message).
##########
airflow-core/src/airflow/cli/commands/connection_command.py:
##########
@@ -411,26 +411,38 @@ def _import_helper(file_path: str, overwrite: bool) ->
None:
:param overwrite: Whether to skip or overwrite on collision.
"""
connections_dict = load_connections_dict(file_path)
+ suc_count = fail_count = 0
with create_session() as session:
for conn_id, conn in connections_dict.items():
try:
helpers.validate_key(conn_id, max_length=200)
except Exception as e:
print(f"Could not import connection. {e}")
+ fail_count += 1
continue
existing_conn_id =
session.scalar(select(Connection.id).where(Connection.conn_id == conn_id))
if existing_conn_id is not None:
if not overwrite:
print(f"Could not import connection {conn_id}: connection
already exists.")
+ fail_count += 1
continue
# The conn_ids match, but the PK of the new entry must also be
the same as the old
conn.id = existing_conn_id
- session.merge(conn)
- session.commit()
- print(f"Imported connection {conn_id}")
+ try:
+ session.merge(conn)
+ session.commit()
+ except Exception as e:
+ print(f"Connection import failed for {conn_id}: {e!r}")
Review Comment:
The error messages are inconsistent (one omits `conn_id`, and the other uses
`repr` formatting via `{e!r}` which can be noisy). For a more consistent CLI
UX, include `conn_id` in both cases and standardize formatting (e.g., include
the exception type + message).
##########
airflow-core/src/airflow/cli/commands/connection_command.py:
##########
@@ -411,26 +411,38 @@ def _import_helper(file_path: str, overwrite: bool) ->
None:
:param overwrite: Whether to skip or overwrite on collision.
"""
connections_dict = load_connections_dict(file_path)
+ suc_count = fail_count = 0
with create_session() as session:
for conn_id, conn in connections_dict.items():
try:
helpers.validate_key(conn_id, max_length=200)
except Exception as e:
print(f"Could not import connection. {e}")
+ fail_count += 1
continue
existing_conn_id =
session.scalar(select(Connection.id).where(Connection.conn_id == conn_id))
if existing_conn_id is not None:
if not overwrite:
print(f"Could not import connection {conn_id}: connection
already exists.")
+ fail_count += 1
continue
# The conn_ids match, but the PK of the new entry must also be
the same as the old
conn.id = existing_conn_id
- session.merge(conn)
- session.commit()
- print(f"Imported connection {conn_id}")
+ try:
+ session.merge(conn)
+ session.commit()
+ except Exception as e:
+ print(f"Connection import failed for {conn_id}: {e!r}")
+ session.rollback()
+ fail_count += 1
+ else:
+ suc_count += 1
Review Comment:
This change removes the per-connection success output (`Imported connection
{conn_id}`), so large imports may appear to 'hang' with no progress feedback
until the end. Consider retaining a per-item success message (or gating it
behind a verbosity flag) while keeping the end summary.
```suggestion
suc_count += 1
print(f"Imported connection {conn_id}")
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
