uranusjr commented on code in PR #31297:
URL: https://github.com/apache/airflow/pull/31297#discussion_r1194501643


##########
airflow/models/xcom.py:
##########
@@ -172,7 +172,7 @@ def set(
         execution_date: datetime.datetime,
         session: Session = NEW_SESSION,
     ) -> None:
-        """:sphinx-autoapi-skip:"""
+        """:sphinx-autoapi-skip:."""

Review Comment:
   ```suggestion
           """Store an XCom value.
           
           :sphinx-autoapi-skip:
           """
   ```
   
   Similar issue with `:meta private:`. Same for everything below.



##########
airflow/utils/sqlalchemy.py:
##########
@@ -51,15 +51,14 @@ class UtcDateTime(TypeDecorator):
     """
     Almost equivalent to :class:`~sqlalchemy.types.TIMESTAMP` with
     ``timezone=True`` option, but it differs from that by:
-
     - Never silently take naive :class:`~datetime.datetime`, instead it
       always raise :exc:`ValueError` unless time zone aware value.
     - :class:`~datetime.datetime` value's :attr:`~datetime.datetime.tzinfo`
       is always converted to UTC.
     - Unlike SQLAlchemy's built-in :class:`~sqlalchemy.types.TIMESTAMP`,
       it never return naive :class:`~datetime.datetime`, but time zone
       aware value, even with SQLite or MySQL.
-    - Always returns TIMESTAMP in UTC
+    - Always returns TIMESTAMP in UTC.
 

Review Comment:
   This should probably be reworded a bit, the new format is even more wrong.



##########
airflow/secrets/environment_variables.py:
##########
@@ -35,7 +35,7 @@ def get_conn_uri(self, conn_id: str) -> str | None:
         """
         Return URI representation of Connection conn_id.
         :param conn_id: the connection id
-        :return: deserialized Connection
+        :return: deserialized Connection.

Review Comment:
   Should add an empty line instead.



##########
airflow/www/fab_security/manager.py:
##########
@@ -1383,7 +1383,7 @@ def _get_user_permission_resources(
         """
         Return a set of resource names with a certain action name
         that a user has access to. Mainly used to fetch all menu permissions
-        on a single db call, will also check public permissions and builtin 
roles
+        on a single db call, will also check public permissions and builtin 
roles.

Review Comment:
   Should add a newline after ``access to.`` instead.



##########
airflow/utils/file.py:
##########
@@ -267,7 +267,7 @@ def find_path_from_directory(
     Recursively search the base path and return the list of file paths that 
should not be ignored.
     :param base_dir_path: the base path to be searched
     :param ignore_file_name: the file name in which specifies the patterns of 
files/dirs to be ignored
-    :param ignore_file_syntax: the syntax of patterns in the ignore file: 
regexp or glob
+    :param ignore_file_syntax: the syntax of patterns in the ignore file: 
regexp or glob.

Review Comment:
   Should add empty line instead



##########
airflow/www/api/experimental/endpoints.py:
##########
@@ -157,7 +157,7 @@ def delete_dag(dag_id):
 def dag_runs(dag_id):
     """
     Returns a list of Dag Runs for a specific DAG ID.
-    :query param state: a query string parameter 
'?state=queued|running|success...'
+    :query param state: a query string parameter 
'?state=queued|running|success...'.

Review Comment:
   Wrong here



##########
airflow/www/extensions/init_appbuilder.py:
##########
@@ -558,18 +566,19 @@ def security_cleanup(self):
         You can use it always or just sometimes to
         perform a security cleanup. Warning this will delete any permission
         that is no longer part of any registered view or menu.
-        Remember invoke ONLY AFTER YOU HAVE REGISTERED ALL VIEWS
+        Remember invoke ONLY AFTER YOU HAVE REGISTERED ALL VIEWS.
         """
         self.sm.security_cleanup(self.baseviews, self.menu)
 
     def security_converge(self, dry=False) -> dict:
         """
-            This method is useful when you use:
-            - `class_permission_name`
-            - `previous_class_permission_name`
-            - `method_permission_name`
-            - `previous_method_permission_name`
-            migrates all permissions to the new names on all the Roles
+        This method is useful when you use:
+        - `class_permission_name`
+        - `previous_class_permission_name`
+        - `method_permission_name`
+        - `previous_method_permission_name`
+        Migrates all permissions to the new names on all the Roles.

Review Comment:
   ```suggestion
           Migrates all permissions to the new names on all the Roles.
   
           This method is useful when you use:
   
           - `class_permission_name`
           - `previous_class_permission_name`
           - `method_permission_name`
           - `previous_method_permission_name`
   ```



##########
airflow/www/fab_security/manager.py:
##########
@@ -588,14 +588,14 @@ def get_oauth_token_secret_name(self, provider):
         """
         Returns the token_secret name for the oauth provider
         if none is configured defaults to oauth_secret
-        this is configured using OAUTH_PROVIDERS and token_secret
+        this is configured using OAUTH_PROVIDERS and token_secret.

Review Comment:
   I think the period should be put behind ``oauth_secret`` above.



##########
airflow/www/fab_security/sqla/manager.py:
##########
@@ -62,7 +62,7 @@ def __init__(self, appbuilder):
         """
         Class constructor
         param appbuilder:
-            F.A.B AppBuilder main object
+            F.A.B AppBuilder main object.

Review Comment:
   Should add a period ane newline after ``Class constructor``.



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to