airflow-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] XD-DENG commented on a change in pull request #3698: [AIRFLOW-2855] Check Cron Expression Validity in DagBag.process_file()
Date Mon, 06 Aug 2018 07:35:11 GMT
XD-DENG commented on a change in pull request #3698: [AIRFLOW-2855] Check Cron Expression Validity
in DagBag.process_file()
URL: https://github.com/apache/incubator-airflow/pull/3698#discussion_r207796998
 
 

 ##########
 File path: tests/models.py
 ##########
 @@ -1038,6 +1038,21 @@ def test_zip(self):
         dagbag.process_file(os.path.join(TEST_DAGS_FOLDER, "test_zip.zip"))
         self.assertTrue(dagbag.get_dag("test_zip_dag"))
 
+    def test_process_file_cron_validity_check(self):
+        """
+        test if an invalid cron expression
+        as schedule interval can be identified
+        """
+        invalid_dag_files = ["test_invalid_cron.py", "test_zip_invalid_cron.zip"]
+        dagbag = models.DagBag()
+
+        for d in invalid_dag_files:
+            dagbag.process_file(os.path.join(TEST_DAGS_FOLDER, d))
+
+        for d in invalid_dag_files:
+            self.assertTrue(any([d in k and "Invalid Cron expression" in v
 
 Review comment:
   Hi @yrqls21 , great points!
   
   - For your **1st point**, actually you're right. It's not really necessary to re-parse
these two files, i.e. the code below can be moved.
   
   ```
   for d in invalid_dag_files:
       dagbag.process_file(os.path.join(TEST_DAGS_FOLDER, d))
   ``` 
   But I purposely keep them here to make it clear to readers what I'm testing here.
   
   - Your **2nd point** is great. I agree it makes no practical difference here, given `n`
and `m` would never be too big here. But I think I will change this part as well. Always good
to keep performance in mind.
   
   Thanks for your reviewing! 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services

Mime
View raw message