LIN-531 Implement PythonOperatorPerArtifact #748
Conversation
…g between sessions
| @@ -252,7 +252,7 @@ def _get_input_variable_sources(self, pred_nodes) -> Dict[str, Set[str]]: | |||
|
|
|||
| def _get_common_variables( | |||
| self, curr_nc: NodeCollection, pred_nc: NodeCollection | |||
| ) -> Tuple[Set[str], Set[LineaID]]: | |||
| ) -> Tuple[List[str], Set[LineaID]]: | |||
There was a problem hiding this comment.
is there a chance of duplication now that you are returning a list instead of set? do you want to return a list of unique inner vars or are repeats ok?
There was a problem hiding this comment.
In this function no, since we only convert the set to list and sort it at the return statement.
lineapy/plugins/jinja_templates/airflow_dag_PythonOperatorPerSession.jinja
Outdated
Show resolved
Hide resolved
lineapy/plugins/jinja_templates/airflow_dag_PythonOperatorPerArtifact.jinja
Outdated
Show resolved
Hide resolved
lineapy/plugins/pipeline_writers.py
Outdated
| "dag_flavor", AirflowDagFlavor.PythonOperatorPerSession | ||
| ) | ||
| dag_flavor = AirflowDagFlavor[ | ||
| self.dag_config.get("dag_flavor", "PythonOperatorPerSession") |
There was a problem hiding this comment.
why did we go to string instead of the prev enum?
I think you can use strenums here to jump between string and number to refer to an option.
There was a problem hiding this comment.
This is more UI reason. Just like when we specify framework for to_pipeline. It's easier for user to specify a string value of PythonOperatorPerSession than find out the class object.
There was a problem hiding this comment.
Is strenums in 3.7 standard library?
| definition. | ||
| """ | ||
| input_var_loading_block = [ | ||
| f"{var} = pickle.load(open('/tmp/{pipeline_name}/variable_{var}.pickle','rb'))" |
There was a problem hiding this comment.
if this is a public method, we'll need to validate pipeline_name. one way to do this is use aubhro's slugify method that he added to utils and ensure that the result is same as the input. if not, raise an excpetion that name is invalid or something.
Description
pickle.loadandpickle.dumpif the calculation has input variables and return variablesThere is some potential for refactoring the PythonOperatorPerSession and PythonOperatorPerArtifact; however, I think it will make more sense to do it when implementing the DockerOperator.
Fixes # (issue)
LIN-531
Type of change
How Has This Been Tested?