Lee-W commented on code in PR #53726:
URL: https://github.com/apache/airflow/pull/53726#discussion_r2251200424
##########
airflow-core/src/airflow/cli/commands/variable_command.py:
##########
@@ -81,20 +81,25 @@ def variables_import(args, session):
"""Import variables from a given file."""
if not os.path.exists(args.file):
raise SystemExit("Missing variables file.")
- with open(args.file) as varfile:
- try:
- var_json = json.load(varfile)
- except JSONDecodeError:
- raise SystemExit("Invalid variables file.")
+
+ try:
+ # Use load_variables which supports JSON, YAML, and ENV formats
+ var_dict = load_variables(args.file)
+ except Exception as e:
Review Comment:
> Enhance LocalFilesystemBackend.load_variables() to support descriptions
ideally this
##########
airflow-core/src/airflow/cli/commands/variable_command.py:
##########
@@ -81,20 +81,25 @@ def variables_import(args, session):
"""Import variables from a given file."""
if not os.path.exists(args.file):
raise SystemExit("Missing variables file.")
- with open(args.file) as varfile:
- try:
- var_json = json.load(varfile)
- except JSONDecodeError:
- raise SystemExit("Invalid variables file.")
+
+ try:
+ # Use load_variables which supports JSON, YAML, and ENV formats
+ var_dict = load_variables(args.file)
+ except Exception as e:
Review Comment:
> parsing level exceptions yaml.MarkedYAMLError, JSONDecodeError handled in
parser functions _parse_yaml_file and _parse_json_file respectively and
transformed into AirflowFileParseException later. As for FileNotFound and other
OSErrors they are handled directly in variables_import function.
Yep, but fail early is still better
##########
airflow-core/src/airflow/cli/commands/variable_command.py:
##########
@@ -81,20 +81,25 @@ def variables_import(args, session):
"""Import variables from a given file."""
if not os.path.exists(args.file):
raise SystemExit("Missing variables file.")
- with open(args.file) as varfile:
- try:
- var_json = json.load(varfile)
- except JSONDecodeError:
- raise SystemExit("Invalid variables file.")
+
+ try:
+ # Use load_variables which supports JSON, YAML, and ENV formats
+ var_dict = load_variables(args.file)
+ except Exception as e:
Review Comment:
> Happy to tackle the description functionality improvements if you're
interested and can point me in the right direction.
Not that familiar with this piece, but I would try to see how I can help
when I have more bandwidth. Thanks!
##########
airflow-core/src/airflow/cli/commands/variable_command.py:
##########
@@ -81,11 +86,20 @@ def variables_import(args, session):
"""Import variables from a given file."""
if not os.path.exists(args.file):
raise SystemExit("Missing variables file.")
- with open(args.file) as varfile:
- try:
- var_json = json.load(varfile)
- except JSONDecodeError:
- raise SystemExit("Invalid variables file.")
+
+ try:
+ var_json = load_variables(args.file)
+ except AirflowUnsupportedFileTypeException as e:
+ raise SystemExit(str(e))
+ except AirflowFileParseException as e:
+ raise SystemExit(str(e))
+ except VariableNotUnique as e:
+ raise SystemExit(str(e))
Review Comment:
```suggestion
except (AirflowUnsupportedFileTypeException, AirflowFileParseException,
VariableNotUnique) as e:
raise SystemExit(str(e))
```
--
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]
