Skip to content

[AIRFLOW-823] Allow specifying execution date in task_info API#2045

Closed
robin-miller-ow wants to merge 1 commit intomasterfrom
unknown repository
Closed

[AIRFLOW-823] Allow specifying execution date in task_info API#2045
robin-miller-ow wants to merge 1 commit intomasterfrom
unknown repository

Conversation

@robin-miller-ow
Copy link
Contributor

Dear Airflow Maintainers,

Please accept this PR that addresses the following issues:

Testing Done:

  • Unittests added to cover this code change.

@bolkedebruin
Copy link
Contributor

bolkedebruin commented Feb 2, 2017

@robin-miller-ow I think this is the wrong approach to make the change. A DAG does not have an execution date, but a DagRun does. A Task does not have an execution_date but a TaskInstance does by inheriting it from the DagRun.

So I would expect:
/dags/<string:dag_id>/<datetime:execution_date>/tasks/<string:task_id>

to return the information you are looking for.

@robin-miller-ow
Copy link
Contributor Author

@bolkedebruin That seems inconsistent to me. When I added a date into the tigger dag API, you said it should be sent in the data, not the URL, so I presume we'd still want to do that. If this is not the right URL, then based on the trigger dag URL, would:
/dags/<string:dag_id>/dag_runs/task_instance/<string:task_id>
be more consistent? Possibly:
/dags/<string:dag_id>/dag_runs/<datetime:execution_date>/task_instance/<string:task_id>
but I would then expect the be posting to
/dags/<string:dag_id>/dag_runs/<datetime:execution_date>
to create a dag run at a specific time. Thoughts?

@bolkedebruin
Copy link
Contributor

@robin-miller-ow that is actually unrelated: GET vs POST. So no that is not more consistent

@codecov-io
Copy link

codecov-io commented Feb 8, 2017

Codecov Report

Merging #2045 into master will increase coverage by 0.22%.
The diff coverage is 96.72%.

@@            Coverage Diff             @@
##           master    #2045      +/-   ##
==========================================
+ Coverage   66.94%   67.16%   +0.22%     
==========================================
  Files         140      142       +2     
  Lines       10635    10688      +53     
==========================================
+ Hits         7120     7179      +59     
+ Misses       3515     3509       -6
Impacted Files Coverage Δ
airflow/www/api/experimental/endpoints.py 97.4% <100%> (+0.73%)
airflow/api/common/experimental/get_task.py 100% <100%> (ø)
...rflow/api/common/experimental/get_task_instance.py 90.9% <90.9%> (ø)
airflow/models.py 86.31% <ø> (+0.15%)
airflow/jobs.py 73.01% <ø> (+0.52%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b0ae70d...738ed86. Read the comment docs.

@robin-miller-ow
Copy link
Contributor Author

@bolkedebruin I think this is now as you would like. Please let me know if there are any further issues.

@bolkedebruin
Copy link
Contributor

LGTM

@asfgit asfgit closed this in 3f546e2 May 12, 2017
alekstorm pushed a commit to alekstorm/incubator-airflow that referenced this pull request Jun 1, 2017
Closes apache#2045 from robin-miller-
ow/release/API_TaskInstanceInfo
stverhae pushed a commit to stverhae/incubator-airflow that referenced this pull request Jun 6, 2017
Closes apache#2045 from robin-miller-
ow/release/API_TaskInstanceInfo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants