Nice job Kamil, I really like where this is going - especially in regards to Variable and XCOM functionality.
With regards to the Task Instance and XCOM endpoints, I was wondering what everyone's thoughts would be on using 'dag_run_id' as a unique identifier as opposed to the existing 'execution_date'? From a client side perspective, I think it would be easier to handle an ID versus a date. I'm basing this off of the below Jira issue that says dag_id and run_id are unique constraints (correct me if I'm wrong about this). I understand there would also be compatibility issues between this change and the experimental API. https://issues.apache.org/jira/browse/AIRFLOW-5593 For the below methods, what about returning a list of IDs as opposed to the full models? You can return a higher number of records with a smaller amount data transfer. The user could then get the full model with individual requests. GET /dags GET /dags/{dag_id}/tasks Personally, I've never used connexion before, however, it seems quite interesting to me. This yaml file would serve not only as a documentation spec, but also as API configuration? I see connexion provides response validation based off of the yaml configuration and ability to override the validator. However - my concerns are similar to Ash's, how easy would it be to maintain this yaml and maintain synchronization between docs and functionality? https://connexion.readthedocs.io/en/latest/response.html#response-validation Best, Matt On Fri, Feb 28, 2020 at 3:09 PM Ash Berlin-Taylor <a...@apache.org> wrote: > An easier URL to view the spec > https://editor.swagger.io/?url=https://raw.githubusercontent.com/PolideaInternal/airflow/1e3e444d03c2e08d8fcee274d9e745a3d3ddeca8/openapi.yaml > > On Feb 28 2020, at 4:01 pm, Ash Berlin-Taylor <a...@apache.org> wrote: > > To view the API spec < > https://github.com/PolideaInternal/airflow/blob/1e3e444d03c2e08d8fcee274d9e745a3d3ddeca8/openapi.yaml> > in an easier to consume form I used https://editor.swagger.io/ and > imported from the (raw) github URL > > Some comments on the proposed OpenAPI spec; > > > > connection id's are not unique, so get/delete might operate on multiple > entries. (This has caused some confusion in the past, but is probably a > separate issue to adding a REST API) > > dag_id is listed as "integer" type. it should be string. Same applies to > most other id types it appears. > > > > PATCH dag: fileloc shouldn't be writable (it's a security risk), > is_active and is_subdag are an internal field and shouldn't be writable > either. > > > > PATCH dag: - Did you think about/rule out having separate > /dag/{dag_id}/pause and unpause methods? > > > > I don't think taskReschedule should be exposed directly at all - it's an > internal detail for sensors in a specific state. > > > > PATCH variables - update_mask parameter: The way I've seen other APIs do > this is PATCH vs PUT requests. PATCH should only update the specified > fields, PUT is "here is the entire representation, make it look like this" > > > > PATCH variables - is_encrypted is exposed. It shouldn't be. It is not > user-setable (it has is not writable in the UI anymore) > > > > GET xComValues: This endpoint support reading resources across multiple > Task Instancce by specify a "-" as a !! - as a separator is not going to > work. That is a valid character in task ids. > > > > xComValues -- I feel the "values" is out of place/not needed. > > > > > > > > -ash > > (The thought of maintaining that swagger/openapi spec file is honestly a > bit daunting and making me lean more towards adding spec validation to FAB > than using connexion - specifically because it's a: huge and b: lives > separately to the code/endpoints) > > On Feb 28 2020, at 3:15 pm, Ash Berlin-Taylor <a...@apache.org> wrote: > > > Great work Kamil, I can't wait till we have a fully featured API in > Airflow! > > > > > > So AIP-13 has been voted on and "accepted". Do you have a proposal for > what we should do with that AIP that we've already voted upon? > > > In the permissions section you talk about creating, for example > "can_edit_variable" -- but on which view? Permissions in FAB are a on a > specific view -- see > https://github.com/dpgaspar/Flask-AppBuilder/blob/120bc3ca38c4559a0099fe3b1a1badb319b6546e/flask_appbuilder/security/sqla/models.py#L71-L78 > > > "There are not many FAB REST API experts" -- there aren't that many > connexion experts either - has anyone involved with the project used > connexion before?. It's a wildly inaccurate metric, but > https://hugovk.github.io/top-pypi-packages/ lists FAB in position 980 and > connexion in 1146, so they are both about as popular as each other. > > > > > > On Feb 26 2020, at 4:40 pm, Kamil Breguła <kamil.breg...@polidea.com> > wrote: > > > > Hello, > > > > > > > > I just created "AIP-32 - Airflow REST API" proposal and would love > > > > community feedback and comments. > > > > > https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-32%3A+Airflow+REST+API > > > > I would love to know what is your expectation from the API. > > > > > > > > We currently have one experimental API, but despite its existence for > > > > 2 years, it has not reached a stable level. There are many critical > > > > aspects to this implementation including no defined schema, very > > > > narrow range of functions, unsafe default configuration, no HATEOAS, > > > > and many others. > > > > > > > > In the past, Drewstone began work on REST API based on OpenAPI. It's > > > > described by AIP-13: OpenAPI 3 based API definition. However, it was > > > > not completed due to the lack of interest from the author and the > > > > Kerberos configuration problem (It was at a time when Breeze was > still > > > > developing, so configuring all dependencies, including Kerberos, was > a > > > > problem). It had a narrow range of features that are expected by > users > > > > e.g. access to XCOM data and management for connection is missing, > > > > > > > > The Polidea and Google teams together with the community want to make > > > > another attempt based on our and community experience. Airflow > > > > deserves new stable solutions. > > > > > > > > Any comments and feedback/discussion in the AIP-32 document are > welcome! > > > > Best regards, > > > > Kamil Breguła > > > > > > > > > > >
