Hi Bolke,
Thanks for the quick and detailed feedback! We will get on with the necessary changes soon. Cheers, Luke Maycock OLIVER WYMAN [email protected]<mailto:[email protected]> www.oliverwyman.com<http://www.oliverwyman.com/> ________________________________ From: Bolke de Bruin <[email protected]> Sent: 05 December 2016 15:03 To: [email protected] Subject: Re: API additions Het Luke, That’s great news and excellent that your are building on the work of the PR! The functionality you are proposing seem reasonable, obviously not replicating too much would be smart, e.g. extend on trigger_dag rather than introducing a new function that almost does the same. A couple of guidelines that I had in mind: - The API resides in airflow/api/common. This means endpoints (code defined in an endpoint) cannot access the database directly. It should always import from airflow.api.common. This decouples the API definition from its server side implementation. - Client side API must be implemented in airflow.api.client.XXX (JSON + Local). While for now this is not DRY it allows for a smoother migration path away from direct DB access in the CLI. A quick glance over the code that was written by you: - /createdagrun/dag/<string:dag_id>/executiondate/<string:execution_date> —> this is indeed trigger_dag, with an extra field in the JSON - /taskstate/dag/<string:dag_id>/task/<string:task_id>/executiondate/<string:execution_date> —> already implemented as task_info might need extending - make sure to use nouns and proper HTTP actions (PUT, GET, POST). For some best practices have a look at http://www.vinaysahni.com/best-practices-for-a-pragmatic-restful-api <http://www.vinaysahni.com/best-practices-for-a-pragmatic-restful-api> In addition, considering the work Max is doing with Flask Application Builder I expect the “endpoints” to change in the future. That makes it even more important to make sure your DB access code resides in airflow.api.common. Also some rework might be required again, it is marked experimental after all :-). Still, I would like to invite you to implement it as we need the drive of the community to point us where we need to go with this. Cheers Bolke > Op 5 dec. 2016, om 14:31 heeft Maycock, Luke > <[email protected]> het volgende geschreven: > > Hello, > > We've had a requirement for Airflow to be able to start DAG Runs and check on > the state of tasks in a specific DAG Run via API. We'd seen that an > experimental API had been put in place and expanded on this, but there has > since been a significant overhaul, mostly around security and the setup in > this PR: https://github.com/apache/incubator-airflow/pull/1783 > > Before we rework our additions > (https://github.com/owlabs/incubator-airflow/pull/11, > https://github.com/owlabs/incubator-airflow/pull/11, > https://github.com/owlabs/incubator-airflow/pull/14) , we thought it might be > worth asking if anybody had any thoughts about what we've done so far and > whether we're treading on anybody else's toes with these features. > > The endpoints that we've written are: > > - Get details of a task instance > - Create a DAG Run (this is somewhat covered by the trigger_dag endpoint that > has now been added, but we also have a version to allow you to specify the > execution date) > - Add a value to xcom for a task instance (initially required the task to > exist, but we had a use to be able to add xcom for tasks that didn't exist, > so we removed that restriction) > > Is anybody else working on these endpoints or any other significant changes > to this area that we'd do better to wait for before we rework these for PR? > > Thanks, > Luke Maycock > OLIVER WYMAN > [email protected]<mailto:[email protected]> > www.oliverwyman.com<http://www.oliverwyman.com/> > > > ________________________________ > This e-mail and any attachments may be confidential or legally privileged. If > you received this message in error or are not the intended recipient, you > should destroy the e-mail message and any attachments or copies, and you are > prohibited from retaining, distributing, disclosing or using any information > contained herein. Please inform us of the erroneous delivery by return > e-mail. Thank you for your cooperation. ________________________________ This e-mail and any attachments may be confidential or legally privileged. If you received this message in error or are not the intended recipient, you should destroy the e-mail message and any attachments or copies, and you are prohibited from retaining, distributing, disclosing or using any information contained herein. Please inform us of the erroneous delivery by return e-mail. Thank you for your cooperation.
