Skip to content

Commit

Permalink
Add support for runtime_type
Browse files Browse the repository at this point in the history
  • Loading branch information
@ptitzler
ptitzler committed Nov 9, 2021
1 parent f662285 commit 4f882b1
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 35 deletions.
10 changes: 5 additions & 5 deletions ...irflow-example-components-connector/airflow_examples_connector/elyra-airflow-catalog.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@
"type": "string",
"default": "Example pipeline components for Apache Airflow"
},
"runtime": {
"title": "Runtime",
"runtime_type": {
"title": "Runtime type",
"description": "The runtime for which to load the example components.",
"type": "string",
"enum": ["airflow"],
"default": "airflow",
"enum": ["APACHE_AIRFLOW"],
"default": "APACHE_AIRFLOW",
"uihints": {
"field_type": "dropdown"
}
Expand All @@ -65,7 +65,7 @@
}
}
},
"required": ["runtime"]
"required": ["runtime_type"]
}
},
"required": ["schema_name", "display_name", "version", "metadata"]
Expand Down
33 changes: 21 additions & 12 deletions ...ors/airflow-example-components-connector/airflow_examples_connector/examples_connector.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@


from elyra.pipeline.catalog_connector import ComponentCatalogConnector
from elyra.pipeline.runtime_type import RuntimeProcessorType


class ExamplesCatalogConnector(ComponentCatalogConnector):
Expand All @@ -36,25 +37,28 @@ def get_catalog_entries(self, catalog_metadata: Dict[str, Any]) -> List[Dict[str
"""
component_list = []

runtime_type = catalog_metadata.get('runtime')
runtime_type_name = catalog_metadata.get('runtime_type')
runtime_type = RuntimeProcessorType.get_instance_by_name(runtime_type_name)
# The runtime type's user-friendly display name
runtime_type_display_name = runtime_type.value

if runtime_type != 'airflow':
self.log.error(f'Cannot retrieve component list for runtime type \'{runtime_type}\': '
'Only Apache Airflow is supported.')
if runtime_type != RuntimeProcessorType.APACHE_AIRFLOW:
self.log.error(f'Cannot retrieve component list for runtime type \'{runtime_type_display_name}\': '
f'Only \'{RuntimeProcessorType.APACHE_AIRFLOW.value}\' is supported.')

This comment has been minimized.

Sign in to view

Copy link
@kevin-bates

kevin-bates Nov 9, 2021

Member

Tisk, tisk, don't trust schema validation do you? 😄

Do you think its going to be confusing to the user to see "Only 'Apache AIrflow' is supported." when their only option is 'APACHE_AIRFLOW'. Just wondering if we should stick with the "constant looking" form of the enum here.

This comment has been minimized.

Sign in to view

Copy link
@ptitzler

ptitzler Nov 9, 2021

Author Member

trust schema validation
You are right. I guess I should remove this code. This is in essence a carry-over from the original version.

confusing to the user to see "Only 'Apache AIrflow' is supported." when their only option is 'APACHE_AIRFLOW'.

In general I agree, but 1) this is mostly debug logging and 2) once the GUI supports display the "display name" the output will be in synch.
# return empty component specification list
return component_list

try:
root_dir = Path(__file__).parent / 'resources'
self.log.debug(f'Retrieving component list for runtime type \'{runtime_type}\' from '
f'{root_dir}')
self.log.debug(f'Retrieving component list for runtime type \'{runtime_type_display_name}\' from '
f'{root_dir}')
pattern = '**/*.py'
self.log.debug(f'Component file pattern: {pattern}')
for file in root_dir.glob(pattern):
component_list.append({'component-id': str(file)[len(str(root_dir)) + 1:]})
self.log.debug(f'Component list: {component_list}')
except Exception as ex:
self.log.error(f"Error retrieving component list for runtime type '{runtime_type}'"
self.log.error(f"Error retrieving component list for runtime type '{runtime_type_display_name}'"
f" from {root_dir}: {ex}")

return component_list
Expand All @@ -79,15 +83,20 @@ def read_catalog_entry(self,
'A component id must be provided.')
return None

runtime_type = catalog_metadata.get('runtime')
if runtime_type != 'airflow':
self.log.error(f'Cannot retrieve component list for runtime type \'{runtime_type}\': '
'Only Apache Airflow is supported.')
return None
runtime_type_name = catalog_metadata.get('runtime_type')
runtime_type = RuntimeProcessorType.get_instance_by_name(runtime_type_name)
# The runtime type's user-friendly display name
runtime_type_display_name = runtime_type.value

if runtime_type != RuntimeProcessorType.APACHE_AIRFLOW:
self.log.error(f'Cannot retrieve component for runtime type \'{runtime_type_name}\': '
f'Only \'{RuntimeProcessorType.APACHE_AIRFLOW.value}\' is supported.')

This comment has been minimized.

Sign in to view

Copy link
@kevin-bates

kevin-bates Nov 9, 2021

Member

This code is nearly identical to that in the other method. It might be best to move this into a helper.

This comment has been minimized.

Sign in to view

Copy link
@kevin-bates

kevin-bates Nov 9, 2021

Member

Did you mean to let the code fall through here and try to continue the operation? If so, perhaps a comment indicating that intention would be good for future maintainers.

This comment has been minimized.

Sign in to view

Copy link
@ptitzler

ptitzler Nov 9, 2021

Author Member

This is a bug! That said, I'll remove this code and start trusting schema validation (:

try:
# load component from resources directory
root_dir = Path(__file__).parent / 'resources'
self.log.debug(f'Retrieving component of runtime type \'{runtime_type_display_name}\' from '
f'{root_dir}')
with open(root_dir / component_id, 'r') as fp:
return fp.read()
except Exception as e:
Expand Down
10 changes: 5 additions & 5 deletions ...connectors/kfp-example-components-connector/kfp_examples_connector/elyra-kfp-catalog.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@
"type": "string",
"default": "Example pipeline components for Kubeflow Pipelines"
},
"runtime": {
"title": "Runtime",
"runtime_type": {
"title": "Runtime type",
"description": "The runtime for which to load the example components.",
"type": "string",
"enum": ["kfp"],
"default": "kfp",
"enum": ["KUBEFLOW_PIPELINES"],
"default": "KUBEFLOW_PIPELINES",
"uihints": {
"field_type": "dropdown"
}
Expand All @@ -65,7 +65,7 @@
}
}
},
"required": ["runtime"]
"required": ["runtime_type"]
}
},
"required": ["schema_name", "display_name", "version", "metadata"]
Expand Down
34 changes: 21 additions & 13 deletions ...-connectors/kfp-example-components-connector/kfp_examples_connector/examples_connector.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
from typing import List
from typing import Optional


from elyra.pipeline.catalog_connector import ComponentCatalogConnector
from elyra.pipeline.runtime_type import RuntimeProcessorType


class ExamplesCatalogConnector(ComponentCatalogConnector):
Expand All @@ -36,25 +36,28 @@ def get_catalog_entries(self, catalog_metadata: Dict[str, Any]) -> List[Dict[str
"""
component_list = []

runtime_type = catalog_metadata.get('runtime')
runtime_type_name = catalog_metadata.get('runtime_type')
runtime_type = RuntimeProcessorType.get_instance_by_name(runtime_type_name)
# The runtime type's user-friendly display name
runtime_type_display_name = runtime_type.value

if runtime_type != 'kfp':
self.log.error(f'Cannot retrieve component list for runtime type \'{runtime_type}\': '
'Only Kubeflow Pipelines is supported.')
if runtime_type != RuntimeProcessorType.KUBEFLOW_PIPELINES:
self.log.error(f'Cannot retrieve component list for runtime type \'{runtime_type_display_name}\': '
f'Only \'{RuntimeProcessorType.KUBEFLOW_PIPELINES.value}\' is supported.')
# return empty component specification list
return component_list

try:
root_dir = Path(__file__).parent / 'resources'
self.log.debug(f'Retrieving component list for runtime type \'{runtime_type}\' from '
f'{root_dir}')
self.log.debug(f'Retrieving component list for runtime type \'{runtime_type_display_name}\' from '
f'{root_dir}')
pattern = '**/*.yaml'
self.log.debug(f'Component file pattern: {pattern}')
for file in root_dir.glob(pattern):
component_list.append({'component-id': str(file)[len(str(root_dir)) + 1:]})
self.log.debug(f'Component list: {component_list}')
except Exception as ex:
self.log.error(f"Error retrieving component list for runtime type '{runtime_type}'"
self.log.error(f"Error retrieving component list for runtime type '{runtime_type_display_name}'"
f" from {root_dir}: {ex}")

return component_list
Expand All @@ -79,15 +82,20 @@ def read_catalog_entry(self,
'A component id must be provided.')
return None

runtime_type = catalog_metadata.get('runtime')
if runtime_type != 'kfp':
self.log.error(f'Cannot retrieve component list for runtime type \'{runtime_type}\': '
'Only Kubeflow Pipelines is supported.')
return None
runtime_type_name = catalog_metadata.get('runtime_type')
runtime_type = RuntimeProcessorType.get_instance_by_name(runtime_type_name)
# The runtime type's user-friendly display name
runtime_type_display_name = runtime_type.value

if runtime_type != RuntimeProcessorType.KUBEFLOW_PIPELINES:
self.log.error(f'Cannot retrieve component for runtime type \'{runtime_type_display_name}\': '
f'Only \'{RuntimeProcessorType.KUBEFLOW_PIPELINES.value}\' is supported.')

This comment has been minimized.

Sign in to view

Copy link
@kevin-bates

kevin-bates Nov 9, 2021

Member

Same comments apply as above (helper method, continuation comment).

This comment has been minimized.

Sign in to view

Copy link
@ptitzler

ptitzler Nov 9, 2021

Author Member

See above. Code will be removed

try:
# load component from resources directory
root_dir = Path(__file__).parent / 'resources'
self.log.debug(f'Retrieving component of runtime type \'{runtime_type_display_name}\' from '
f'{root_dir}')
with open(root_dir / component_id, 'r') as fp:
return fp.read()
except Exception as e:
Expand Down

0 comments on commit 4f882b1

Please sign in to comment.