Conversation
mingjerli
changed the title
yoonspark
force-pushed
the
implement-pipeline-writers-for-artifact-collection
branch
from
d52f60d to
2cba35d
Compare
|
General comment, how were the "expected" files generated, and how did we validate that they are correct? |
|
@andycui97 The "expected" files were generated by running the test scenarios over local Jupyter notebook sessions; after manually inspecting/testing they look good and runnable, these files were copied to the test folder as "expected" results. |
yoonspark
force-pushed
the
implement-pipeline-writers-for-artifact-collection
branch
from
72c90f4 to
2947ee5
Compare
| lineapy==0.1.4 | ||
| lineapy==0.1.4 |
There was a problem hiding this comment.
why are there duplicates here? i think we ensure that we dont have duplicate ones. also for lineapy the plugins dont include the exact version for lineapy (the tests break every time we try to release because the versions wont match)
There was a problem hiding this comment.
Looks like it is concatenating requirements from each session. In this case, it is from two sessions, so it is duplicated. I've fixed the problem.
However, it actually raises an interesting question. What should we do if the requirements for two sessions conflict?
lionsardesai
changed the base branch from
main
to
158-userdef-classmethods-mutate
lionsardesai
changed the base branch from
158-userdef-classmethods-mutate
to
main
| dag = DAG( | ||
| dag_id="{{ DAG_NAME }}_dag", | ||
| schedule_interval="{{ SCHEDULE_INTERVAL }}", | ||
| max_active_runs={{ MAX_ACTIVE_RUNS }}, | ||
| catchup={{ CATCHUP }}, | ||
| default_args=default_dag_args, | ||
| ) |
There was a problem hiding this comment.
could simplify this to use context manager that auto adds dags to the operators. see: https://airflow.apache.org/docs/apache-airflow/stable/tutorial.html
if seems that using context manager is preferred but i leave it to @mingjerli and @andycui97 to determine if this is backwards compatible to sufficiently old airflow version.
lionsardesai
left a comment
lionsardesai
left a comment
There was a problem hiding this comment.
talked to @yoonspark, and holding off the merge till changes are done.
| keep_lineapy_save: bool = False, | ||
| pipeline_name: str = "pipeline", | ||
| output_dir: str = ".", | ||
| dag_config: Optional[AirflowDagConfig] = {}, |
There was a problem hiding this comment.
we should generalize this later. for now we can proceed as is.
yoonspark
changed the title
yoonspark
deleted the
implement-pipeline-writers-for-artifact-collection
branch
5 participants
Description
Implement pipeline writers for SCRIPT (LIN-477) and AIRFLOW (LIN-484) frameworks. These writers use new graph refactoring implemented by
ArtifactCollectionfor pipeline file generation; and they are meant to eventually replace existing plugins.NOTE: This PR "absorbs" #737.
Type of change
How Has This Been Tested?
New unit tests were added to validate if pipeline files are properly generated.