You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/02/26 22:48:43 UTC
[GitHub] [airflow] ephraimbuddy opened a new pull request #14490: Bugfix: Fix wrong output of tags and owners in dag detail API endpoint
ephraimbuddy opened a new pull request #14490:
URL: https://github.com/apache/airflow/pull/14490
Closes: https://github.com/apache/airflow/issues/14481
The dag detail endpoint is a python object and was modelled as a database schema object. This caused the tags output to be empty because the tags attribute of DAG object is just a string while DagModel tags attribute is a list of TagModel object.
Also, DAG object doesn't have `owners` attribute but `owner` which is a string containing the owners e.g `owner = 'airflow, Anotherowner'`.
This PR fixes this by modeling the DagDetail schema as a python object.
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14490: Bugfix: Fix wrong output of tags and owners in dag detail API endpoint
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14490:
URL: https://github.com/apache/airflow/pull/14490#discussion_r584478774
##########
File path: tests/api_connexion/schemas/test_dag_schema.py
##########
@@ -122,11 +123,11 @@ def test_serialize(self):
'is_paused': None,
'is_subdag': False,
'orientation': 'LR',
- 'owners': [],
+ 'owner': '',
'params': {'foo': 1},
'schedule_interval': {'__type': 'TimeDelta', 'days': 1, 'seconds': 0, 'microseconds': 0},
'start_date': '2020-06-19T00:00:00+00:00',
- 'tags': None,
+ 'tags': ["example1", "example2"],
Review comment:
Previously, the endpoint returns tags object and that made it to output empty objects {} because in the DAG object, tags is a list of strings unlike DagModel where tags is an object.
I think, since this endpoint only outputs data and doesn't add, we will not worry about adding anything else. I have removed the connection between the schema with DagSchema where it inherited DagSchema which defined tags.
Now the schema is standing alone and not connected with other schemas, and only for the Dagdetails endpoint which only outputs data.
What do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #14490: Bugfix: Fix wrong output of tags and owners in dag detail API endpoint
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14490:
URL: https://github.com/apache/airflow/pull/14490#issuecomment-789057394
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest master or amend the last commit of the PR, and push it with --force-with-lease.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14490: Bugfix: Fix wrong output of tags and owners in dag detail API endpoint
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14490:
URL: https://github.com/apache/airflow/pull/14490#discussion_r585042477
##########
File path: tests/api_connexion/schemas/test_dag_schema.py
##########
@@ -122,11 +123,11 @@ def test_serialize(self):
'is_paused': None,
'is_subdag': False,
'orientation': 'LR',
- 'owners': [],
+ 'owner': '',
'params': {'foo': 1},
'schedule_interval': {'__type': 'TimeDelta', 'days': 1, 'seconds': 0, 'microseconds': 0},
'start_date': '2020-06-19T00:00:00+00:00',
- 'tags': None,
+ 'tags': ["example1", "example2"],
Review comment:
Fixed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy merged pull request #14490: Bugfix: Fix wrong output of tags and owners in dag detail API endpoint
Posted by GitBox <gi...@apache.org>.
ephraimbuddy merged pull request #14490:
URL: https://github.com/apache/airflow/pull/14490
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #14490: Bugfix: Fix wrong output of tags and owners in dag detail API endpoint
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #14490:
URL: https://github.com/apache/airflow/pull/14490#discussion_r585042811
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1940,39 +1940,84 @@ components:
For details see:
(airflow.models.DAG)[https://airflow.apache.org/docs/apache-airflow/stable/_api/airflow/models/index.html#airflow.models.DAG]
- allOf:
- - $ref: '#/components/schemas/DAG'
- - type: object
- properties:
- timezone:
- $ref: '#/components/schemas/Timezone'
- catchup:
- type: boolean
- readOnly: true
- orientation:
- type: string
- readOnly: true
- concurrency:
- type: number
- readOnly: true
- start_date:
- type: string
- format: 'date-time'
- readOnly: true
- nullable: true
- dag_run_timeout:
- nullable: true
- $ref: '#/components/schemas/TimeDelta'
- doc_md:
- type: string
- readOnly: true
- nullable: true
- default_view:
- type: string
- readOnly: true
- params:
- type: object
- readOnly: true
+ properties:
+ dag_id:
+ type: string
+ readOnly: true
+ description: The ID of the DAG.
+ root_dag_id:
+ type: string
+ readOnly: true
+ nullable: true
+ description: If the DAG is SubDAG then it is the top level DAG identifier. Otherwise, nulll.
+ is_paused:
+ type: boolean
+ nullable: true
+ description: Whether the DAG is paused.
+ is_subdag:
+ description: Whether the DAG is SubDAG.
+ type: boolean
+ readOnly: true
+ fileloc:
+ description: The absolute path to the file.
+ type: string
+ readOnly: true
+ file_token:
+ type: string
+ readOnly: true
+ description: >
+ The key containing the encrypted path to the file. Encryption and decryption take place only on
+ the server. This prevents the client from reading an non-DAG file. This also ensures API
+ extensibility, because the format of encrypted data may change.
+ owner:
+ type: string
Review comment:
Fixed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ryanahamilton commented on a change in pull request #14490: Bugfix: Fix wrong output of tags and owners in dag detail API endpoint
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on a change in pull request #14490:
URL: https://github.com/apache/airflow/pull/14490#discussion_r584879689
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1940,39 +1940,84 @@ components:
For details see:
(airflow.models.DAG)[https://airflow.apache.org/docs/apache-airflow/stable/_api/airflow/models/index.html#airflow.models.DAG]
- allOf:
- - $ref: '#/components/schemas/DAG'
- - type: object
- properties:
- timezone:
- $ref: '#/components/schemas/Timezone'
- catchup:
- type: boolean
- readOnly: true
- orientation:
- type: string
- readOnly: true
- concurrency:
- type: number
- readOnly: true
- start_date:
- type: string
- format: 'date-time'
- readOnly: true
- nullable: true
- dag_run_timeout:
- nullable: true
- $ref: '#/components/schemas/TimeDelta'
- doc_md:
- type: string
- readOnly: true
- nullable: true
- default_view:
- type: string
- readOnly: true
- params:
- type: object
- readOnly: true
+ properties:
+ dag_id:
+ type: string
+ readOnly: true
+ description: The ID of the DAG.
+ root_dag_id:
+ type: string
+ readOnly: true
+ nullable: true
+ description: If the DAG is SubDAG then it is the top level DAG identifier. Otherwise, nulll.
+ is_paused:
+ type: boolean
+ nullable: true
+ description: Whether the DAG is paused.
+ is_subdag:
+ description: Whether the DAG is SubDAG.
+ type: boolean
+ readOnly: true
+ fileloc:
+ description: The absolute path to the file.
+ type: string
+ readOnly: true
+ file_token:
+ type: string
+ readOnly: true
+ description: >
+ The key containing the encrypted path to the file. Encryption and decryption take place only on
+ the server. This prevents the client from reading an non-DAG file. This also ensures API
+ extensibility, because the format of encrypted data may change.
+ owner:
+ type: string
Review comment:
Should this remain an array as it is for the other endpoint above (line 1500)?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #14490: Bugfix: Fix wrong output of tags and owners in dag detail API endpoint
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #14490:
URL: https://github.com/apache/airflow/pull/14490#discussion_r584433963
##########
File path: tests/api_connexion/schemas/test_dag_schema.py
##########
@@ -122,11 +123,11 @@ def test_serialize(self):
'is_paused': None,
'is_subdag': False,
'orientation': 'LR',
- 'owners': [],
+ 'owner': '',
'params': {'foo': 1},
'schedule_interval': {'__type': 'TimeDelta', 'days': 1, 'seconds': 0, 'microseconds': 0},
'start_date': '2020-06-19T00:00:00+00:00',
- 'tags': None,
+ 'tags': ["example1", "example2"],
Review comment:
We should return objects to make it more future-proof and extensible. for example, if we want to add a color, we can do so without breaking changes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org