Lee-W commented on code in PR #39138:
URL: https://github.com/apache/airflow/pull/39138#discussion_r1580712852


##########
airflow/models/dagbag.py:
##########
@@ -727,3 +734,41 @@ def _sync_perm_for_dag(cls, dag: DAG, session: Session = 
NEW_SESSION):
 
         security_manager = ApplessAirflowSecurityManager(session=session)
         security_manager.sync_perm_for_dag(root_dag_id, dag.access_control)
+
+
+class DagPriorityParsingRequests(Base):
+    """Model to store the dag parsing requests that will be prioritized when 
parsing files."""
+
+    __tablename__ = "dag_priority_parsing_requests"

Review Comment:
   ```suggestion
       __tablename__ = "dag_priority_parsing_request"
   ```



##########
airflow/dag_processing/manager.py:
##########
@@ -728,6 +730,24 @@ def _add_callback_to_queue(self, request: CallbackRequest):
             self._add_paths_to_queue([request.full_filepath], True)
             Stats.incr("dag_processing.other_callback_count")
 
+    @provide_session
+    def _refresh_requested_filelocs(self, session=NEW_SESSION):
+        """Refresh filepaths from dag dir as requested by users via APIs."""
+        # Get values from DB table
+        requests = DagPriorityParsingRequests.get_requests()
+        for request in requests:
+            # Check if fileloc is in valid file paths. Parsing any
+            # filepaths can be a security issue.

Review Comment:
   if this could be a security issue, should we add a warning?



##########
airflow/dag_processing/manager.py:
##########
@@ -728,6 +730,24 @@ def _add_callback_to_queue(self, request: CallbackRequest):
             self._add_paths_to_queue([request.full_filepath], True)
             Stats.incr("dag_processing.other_callback_count")
 
+    @provide_session
+    def _refresh_requested_filelocs(self, session=NEW_SESSION):

Review Comment:
   ```suggestion
       def _refresh_requested_filelocs(self, session=NEW_SESSION) -> None:
   ```



##########
airflow/models/dagbag.py:
##########
@@ -727,3 +734,41 @@ def _sync_perm_for_dag(cls, dag: DAG, session: Session = 
NEW_SESSION):
 
         security_manager = ApplessAirflowSecurityManager(session=session)
         security_manager.sync_perm_for_dag(root_dag_id, dag.access_control)
+
+
+class DagPriorityParsingRequests(Base):
+    """Model to store the dag parsing requests that will be prioritized when 
parsing files."""
+
+    __tablename__ = "dag_priority_parsing_requests"
+
+    id = Column(String(40), primary_key=True)
+    # The location of the file containing the DAG object
+    # Note: Do not depend on fileloc pointing to a file; in the case of a
+    # packaged DAG, it will point to the subpath of the DAG within the
+    # associated zip.
+    fileloc = Column(String(2000), nullable=False)
+
+    def __init__(self, fileloc: str):

Review Comment:
   ```suggestion
       def __init__(self, fileloc: str) -> None:
   ```



##########
airflow/models/dagbag.py:
##########
@@ -727,3 +734,41 @@ def _sync_perm_for_dag(cls, dag: DAG, session: Session = 
NEW_SESSION):
 
         security_manager = ApplessAirflowSecurityManager(session=session)
         security_manager.sync_perm_for_dag(root_dag_id, dag.access_control)
+
+
+class DagPriorityParsingRequests(Base):
+    """Model to store the dag parsing requests that will be prioritized when 
parsing files."""
+
+    __tablename__ = "dag_priority_parsing_requests"
+
+    id = Column(String(40), primary_key=True)
+    # The location of the file containing the DAG object
+    # Note: Do not depend on fileloc pointing to a file; in the case of a
+    # packaged DAG, it will point to the subpath of the DAG within the
+    # associated zip.
+    fileloc = Column(String(2000), nullable=False)

Review Comment:
   Just notice this `2000` is used in a few places. Should we make it a 
constant?



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