Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix templates for safe rendering of comments, pipeline & property descriptions #2467

Merged
merged 7 commits into from
Feb 16, 2022
Merged
8 changes: 5 additions & 3 deletions elyra/templates/airflow/airflow_template.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ dag = DAG(
default_args=args,
schedule_interval='@once',
start_date=days_ago(1),
description='{{ pipeline_description }}',
description="""
{{ pipeline_description|replace("\"\"\"", "\\\"\\\"\\\"") }}
""",
is_paused_upon_creation={{ is_paused_upon_creation }},
)

Expand Down Expand Up @@ -86,7 +88,7 @@ op_{{ operation.id|regex_replace }} = KubernetesPodOperator(name='{{ operation.n

{% if operation.doc %}
op_{{ operation.id|regex_replace }}.doc = """
{{ operation.doc }}
{{ operation.doc|replace("\"\"\"", "\\\"\\\"\\\"") }}
"""
{% endif %}

Expand All @@ -95,4 +97,4 @@ op_{{ operation.id|regex_replace }} = KubernetesPodOperator(name='{{ operation.n
op_{{ operation.id|regex_replace }} << op_{{ parent_operation_id|regex_replace }}
{% endfor %}
{% endif %}
{% endfor %}
{% endfor %}
4 changes: 2 additions & 2 deletions elyra/templates/components/canvas_properties_template.jinja2
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"current_parameters": {
{% if component.description %}"component_description": "{{ component.description }}",{% endif %}
{% if component.description %}"component_description": {{ component.description|tojson|safe }},{% endif %}
"label": "",
{% for property in component.properties %}
"elyra_{{ property.ref }}":
Expand Down Expand Up @@ -81,7 +81,7 @@
"default": "{{ property.name }}"
},
"description": {
"default": "{{ property.description }}",
"default": {{ property.description|tojson|safe }},
"placement": "on_panel"
},
"data": {
Expand Down
7 changes: 7 additions & 0 deletions elyra/tests/pipeline/airflow/test_component_parser_airflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,13 @@ def get_parameter_required(param_name):
"(type: a list of strings)"
assert get_parameter_description('elyra_fallback_type') == "(type: str)"

# Ensure that a long description with line wrapping and a backslash escape has rendered
# (and hence did not raise an error during json.loads in the properties API request)
parsed_description = """a string parameter with a very long description
that wraps lines and also has an escaped underscore in it, as shown here: (\_) # noqa W605"""
modified_description = parsed_description.replace("\n", " ") + " (type: str)" # modify desc acc. to parser rules
assert get_parameter_description('elyra_long_description_property') == modified_description

# Retrieve properties for DeriveFromTestOperator
# DeriveFromTestOperator includes type hints for all init arguments
properties_json = ComponentCache.to_canvas_properties(derive_test_op)
Expand Down
17 changes: 16 additions & 1 deletion elyra/tests/pipeline/airflow/test_processor_airflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,11 +278,26 @@ def test_create_file_custom_components(monkeypatch, processor, parsed_pipeline,

file_as_lines = open(response).read().splitlines()

# Check DAG project name
pipeline_description = pipeline_json['pipelines'][0]['app_data']['properties']['description']
escaped_description = pipeline_description.replace("\"\"\"", "\\\"\\\"\\\"")

for i in range(len(file_as_lines)):
if "args = {" == file_as_lines[i]:
# Check DAG project name
assert "project_id" == read_key_pair(file_as_lines[i + 1], sep=':')['key']
assert export_pipeline_name == read_key_pair(file_as_lines[i + 1], sep=':')['value']
elif 'description="""' in file_as_lines[i]:
# Check that DAG contains the correct description
line_no = i + 1
description_as_lines = []
while '"""' not in file_as_lines[line_no]:
description_as_lines.append(file_as_lines[line_no])
line_no += 1
expected_description_lines = escaped_description.split("\n")
assert description_as_lines == expected_description_lines

# Nothing more to be done in file
break

# For every node in the original pipeline json
for node in pipeline_json['pipelines'][0]['nodes']:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ class TestOperator(BaseOperator):
:type unusual_type_dict: a dictionary of arrays
:param unusual_type_list: a list parameter with the phrase 'string' in type description
:type unusual_type_list: a list of strings
:param long_description_property: a string parameter with a very long description
that wraps lines and also has an escaped underscore in it, as shown here: (\_) # noqa W605
:type long_description_property: str
"""

def __init__(
Expand All @@ -75,6 +78,7 @@ def __init__(
unusual_type_dict=None,
unusual_type_list=None,
fallback_type=None,
long_description_property=None,
*args,
**kwargs
):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -390,9 +390,16 @@
"ui_data": {
"comments": []
},
"version": 4,
"version": 7,
"runtime": "airflow",
"runtime_config": "airflow-yukked1",
"runtime_type": "APACHE_AIRFLOW",
"properties": {
"name": "pipeline_with_custom_components",
"runtime": "Apache Airflow",
"description": "This is a pipeline description that\nincludes newline characters\n\n\"\"\"Note that it also includes a help string\"\"\""
},
"name": "pipeline_with_custom_components",
"source": "pipeline_with_custom_components.json"
},
"runtime_ref": ""
Expand Down