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 2020/08/20 00:53:09 UTC

[GitHub] [airflow] mik-laj opened a new pull request #10417: Improving descriptions in OpenAPI

mik-laj opened a new pull request #10417:
URL: https://github.com/apache/airflow/pull/10417


   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   ---
   **^ 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] houqp commented on a change in pull request #10417: Improving descriptions in OpenAPI

Posted by GitBox <gi...@apache.org>.
houqp commented on a change in pull request #10417:
URL: https://github.com/apache/airflow/pull/10417#discussion_r475180202



##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1076,7 +1111,7 @@ paths:
 
   /version:
     get:
-      summary: Get version information
+      summary: Get a version information

Review comment:
       would be better to just say `Get current Airflow version` here. It's unclear what `version information` actually means.
   
   in the response section below, we have the description set to `Return current configuration`. Is this still accurate?

##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1052,17 +1064,40 @@ paths:
             application/json:
               schema:
                 $ref: '#/components/schemas/Config'
+              example:
+                sections:
+                  - name: core
+                    options:
+                      - key: dags_folder
+                        value: /home/user/my-dags-folder
+
+                  - name: smtp
+                    options:
+                      - key: smtp_host
+                        value: localhost
+                      - key: smtp_mail_from
+                        value: airflow@example.com
             text/plain:
               schema:
                 type: string
+              example: |
+                [core]
+                dags_folder = /home/user/my-dags-folder
+                [smtp]
+                smtp_host = localhost
+                smtp_mail_from =  airflow@example.com
+
+
         '401':
           $ref: '#/components/responses/Unauthenticated'
         '403':
           $ref: '#/components/responses/PermissionDenied'
 
   /health:
     get:
-      summary: Returns the status of Airflow's metadatabase and scheduler
+      summary: Get a instance status
+      description: |
+        Get the status of Airflow's metadatabase and scheduler.

Review comment:
       I feel like we can just use `Get the health status of Airflow's metadatabase and scheduler` as summary here and remove the description key. `Get an instance status` doesn't really mean anything from an external reader's point of view.




----------------------------------------------------------------
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 merged pull request #10417: Improving descriptions in OpenAPI

Posted by GitBox <gi...@apache.org>.
mik-laj merged pull request #10417:
URL: https://github.com/apache/airflow/pull/10417


   


----------------------------------------------------------------
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] michalslowikowski00 commented on a change in pull request #10417: Improving descriptions in OpenAPI

Posted by GitBox <gi...@apache.org>.
michalslowikowski00 commented on a change in pull request #10417:
URL: https://github.com/apache/airflow/pull/10417#discussion_r473727909



##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1256,65 +1319,75 @@ components:
 
     EventLog:
       type: object
+      description: Log of user operations via CLI or Web UI.
       properties:
         event_log_id:
+          description: The evnet log ID
           type: integer
           readOnly: true
         when:
+          description: The time when these events happened.
+          format: date-time
           type: string
           readOnly: true
         dag_id:
+          description: The DAG ID
           type: string
           readOnly: true
         task_id:
+          description: The DAG ID
           type: string
           readOnly: true
         event:
+          description: A key describing the type of event
           type: string
           readOnly: true
         execution_date:
+          description: |
+            When the event was dispatched for an object having execution_date, the value of this field.
+          format: date-time
           type: string
           readOnly: true
         owner:
+          description: Name of the user who triggered these events a.
           type: string
           readOnly: true
         extra:
+          description: |
+            Other information that was not included in the other fields, e.g. the complete CLI command
           type: string
           readOnly: true
           nullable: true
 
     EventLogCollection:
       type: object
+      description: Collecttion of event log
       properties:
         event_logs:
           type: array
           items:
             $ref: '#/components/schemas/EventLog'
 
-    HealthInfo:
-      type: object
-      properties:
-        metadatabase:
-          $ref: '#/components/schemas/MetadatabaseStatus'
-        scheduler:
-          $ref: '#/components/schemas/SchedulerStatus'
-
     ImportError:
       type: object
       properties:
         import_error_id:
           type: integer
           readOnly: true
+          description: The import error ID
         timestamp:
           type: string
           format: datetime
           readOnly: true
+          description: The time when this error was created,

Review comment:
       ```suggestion
             description: The time when this error was created
   ```




----------------------------------------------------------------
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 #10417: Improving descriptions in OpenAPI

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10417:
URL: https://github.com/apache/airflow/pull/10417#discussion_r475181656



##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1076,7 +1111,7 @@ paths:
 
   /version:
     get:
-      summary: Get version information
+      summary: Get a version information

Review comment:
       This endpoint is in a separate section, so in my opinion it expresses its intention clearly enough. The summary should also not be longer than 20 characters to be accessible to everyone.
   
   I will correct the description for the succesfull response.




----------------------------------------------------------------
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] michalslowikowski00 commented on a change in pull request #10417: Improving descriptions in OpenAPI

Posted by GitBox <gi...@apache.org>.
michalslowikowski00 commented on a change in pull request #10417:
URL: https://github.com/apache/airflow/pull/10417#discussion_r473728458



##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1324,59 +1397,81 @@ components:
           items:
             $ref: '#/components/schemas/ImportError'
 
+    HealthInfo:
+      type: object
+      description: Instance status information.
+      properties:
+        metadatabase:
+          $ref: '#/components/schemas/MetadatabaseStatus'
+        scheduler:
+          $ref: '#/components/schemas/SchedulerStatus'
+
     MetadatabaseStatus:
       type: object
-      description: Returns the status of the metadatabase
+      description: The status of the metadatabase
+      properties:
+        status:
+          $ref: '#/components/schemas/HealthStatus'
+
+    SchedulerStatus:
+      type: object
+      description: The status and the latest scheduler heartbeat
       properties:
         status:
           $ref: '#/components/schemas/HealthStatus'
+        latest_scheduler_heartbeat:
+          description: The time the scheduler last do a heartbeat.
+          type: string
+          format: datetime
+          readOnly: true
+          nullable: true
+
 
     Pool:
+      description: The pool
       type: object
       properties:
         name:
           type: string
+          description: The name of pool.
         slots:
           type: integer
+          description: |
+            The maximum number of slots that can be assigned to tasks. One job may occupy one or more slots.
         occupied_slots:
           type: integer
           readOnly: true
+          description: The number of slots used by running/queued tasks at the moment.
         used_slots:
           type: integer
           readOnly: true
+          description: The number of slots used by running tasks at the moment.
         queued_slots:
           type: integer
           readOnly: true
+          description: The number of slots used by queued tasks at the moment.
         open_slots:
           type: integer
           readOnly: true
+          description: The number of free slots at the moment.
 
     PoolCollection:
       type: object
+      description: Collection of pool
       properties:
         pools:
           type: array
           items:
             $ref: '#/components/schemas/Pool'
 
-    SchedulerStatus:
-      type: object
-      description: Returns the status and the latest scheduler heartbeat
-      properties:
-        status:
-          $ref: '#/components/schemas/HealthStatus'
-        latest_scheduler_heartbeat:
-          type: string
-          format: datetime
-          readOnly: true
-          nullable: true
 
     SLAMiss:
       type: object
       properties:
         task_id:
           type: string
           readOnly: true
+          description: The Task ID

Review comment:
       ```suggestion
             description: The Task ID.
   ```




----------------------------------------------------------------
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] michalslowikowski00 commented on a change in pull request #10417: Improving descriptions in OpenAPI

Posted by GitBox <gi...@apache.org>.
michalslowikowski00 commented on a change in pull request #10417:
URL: https://github.com/apache/airflow/pull/10417#discussion_r473729738



##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1324,59 +1397,81 @@ components:
           items:
             $ref: '#/components/schemas/ImportError'
 
+    HealthInfo:
+      type: object
+      description: Instance status information.
+      properties:
+        metadatabase:
+          $ref: '#/components/schemas/MetadatabaseStatus'
+        scheduler:
+          $ref: '#/components/schemas/SchedulerStatus'
+
     MetadatabaseStatus:
       type: object
-      description: Returns the status of the metadatabase
+      description: The status of the metadatabase
+      properties:
+        status:
+          $ref: '#/components/schemas/HealthStatus'
+
+    SchedulerStatus:
+      type: object
+      description: The status and the latest scheduler heartbeat
       properties:
         status:
           $ref: '#/components/schemas/HealthStatus'
+        latest_scheduler_heartbeat:
+          description: The time the scheduler last do a heartbeat.
+          type: string
+          format: datetime
+          readOnly: true
+          nullable: true
+
 
     Pool:
+      description: The pool
       type: object
       properties:
         name:
           type: string
+          description: The name of pool.
         slots:
           type: integer
+          description: |
+            The maximum number of slots that can be assigned to tasks. One job may occupy one or more slots.
         occupied_slots:
           type: integer
           readOnly: true
+          description: The number of slots used by running/queued tasks at the moment.
         used_slots:
           type: integer
           readOnly: true
+          description: The number of slots used by running tasks at the moment.
         queued_slots:
           type: integer
           readOnly: true
+          description: The number of slots used by queued tasks at the moment.
         open_slots:
           type: integer
           readOnly: true
+          description: The number of free slots at the moment.
 
     PoolCollection:
       type: object
+      description: Collection of pool
       properties:
         pools:
           type: array
           items:
             $ref: '#/components/schemas/Pool'
 
-    SchedulerStatus:
-      type: object
-      description: Returns the status and the latest scheduler heartbeat
-      properties:
-        status:
-          $ref: '#/components/schemas/HealthStatus'
-        latest_scheduler_heartbeat:
-          type: string
-          format: datetime
-          readOnly: true
-          nullable: true
 
     SLAMiss:
       type: object
       properties:
         task_id:
           type: string
           readOnly: true
+          description: The Task ID

Review comment:
       Sometimes there is full stop at the end of the sentence and sometimes isn't.




----------------------------------------------------------------
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 #10417: Improving descriptions in OpenAPI

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10417:
URL: https://github.com/apache/airflow/pull/10417#discussion_r475905968



##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -88,13 +88,13 @@ paths:
       - $ref: '#/components/parameters/ConnectionID'
 
     get:
-      summary: Get a connection entry
+      summary: Get a connection
       x-openapi-router-controller: airflow.api_connexion.endpoints.connection_endpoint
       operationId: get_connection
       tags: [Connection]
       responses:
         '200':
-          description: Successful response.
+          description: Sucess.

Review comment:
       I update the description.




----------------------------------------------------------------
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 #10417: Improving descriptions in OpenAPI

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10417:
URL: https://github.com/apache/airflow/pull/10417#discussion_r475902458



##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -88,13 +88,13 @@ paths:
       - $ref: '#/components/parameters/ConnectionID'
 
     get:
-      summary: Get a connection entry
+      summary: Get a connection
       x-openapi-router-controller: airflow.api_connexion.endpoints.connection_endpoint
       operationId: get_connection
       tags: [Connection]
       responses:
         '200':
-          description: Successful response.
+          description: Sucess.

Review comment:
       Right. This field should describe **when** this response is returned and not contain information on what the field contains. We have other places to describe the content.




----------------------------------------------------------------
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 #10417: Improving descriptions in OpenAPI

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10417:
URL: https://github.com/apache/airflow/pull/10417#discussion_r475181157



##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -1052,17 +1064,40 @@ paths:
             application/json:
               schema:
                 $ref: '#/components/schemas/Config'
+              example:
+                sections:
+                  - name: core
+                    options:
+                      - key: dags_folder
+                        value: /home/user/my-dags-folder
+
+                  - name: smtp
+                    options:
+                      - key: smtp_host
+                        value: localhost
+                      - key: smtp_mail_from
+                        value: airflow@example.com
             text/plain:
               schema:
                 type: string
+              example: |
+                [core]
+                dags_folder = /home/user/my-dags-folder
+                [smtp]
+                smtp_host = localhost
+                smtp_mail_from =  airflow@example.com
+
+
         '401':
           $ref: '#/components/responses/Unauthenticated'
         '403':
           $ref: '#/components/responses/PermissionDenied'
 
   /health:
     get:
-      summary: Returns the status of Airflow's metadatabase and scheduler
+      summary: Get a instance status
+      description: |
+        Get the status of Airflow's metadatabase and scheduler.

Review comment:
       The summary should be no longer than 20 characters to make it accessible to all people. The description can be of any length. I don't think the problem is that the summary doesn't say everything as users can access the full description easily, but the short titles make it easier to navigate.
   
   <img width="300" alt="Screenshot 2020-08-23 at 08 56 27" src="https://user-images.githubusercontent.com/12058428/90973025-8ddbe700-e51e-11ea-8947-b34bfcc4b82e.png">
   https://airflow.readthedocs.io/en/latest/stable-rest-api-ref.html#tag/Monitoring
   




----------------------------------------------------------------
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] turbaszek commented on a change in pull request #10417: Improving descriptions in OpenAPI

Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #10417:
URL: https://github.com/apache/airflow/pull/10417#discussion_r475900281



##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -88,13 +88,13 @@ paths:
       - $ref: '#/components/parameters/ConnectionID'
 
     get:
-      summary: Get a connection entry
+      summary: Get a connection
       x-openapi-router-controller: airflow.api_connexion.endpoints.connection_endpoint
       operationId: get_connection
       tags: [Connection]
       responses:
         '200':
-          description: Successful response.
+          description: Sucess.

Review comment:
       In L49 we use more meaningful description. Should we unify the style?




----------------------------------------------------------------
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