youngyjd commented on issue #5224: [AIRFLOW-4146] Fix CgroupTaskRunner errors URL: https://github.com/apache/airflow/pull/5224#issuecomment-488570199 @feng-tao @ashb We don't have existing tests for CgroupTaskRunner, that's why even there is a package import error in CgroupTaskRunner but it has not been fixed ever. I haven't written any test for cgroups before so don't have an idea. Based on what I understand, cgroup first must be mount on disk first and `cgroup-bin` Ubuntu package must be installed, otherwise https://github.com/apache/airflow/blob/9e72767b8e4de7f5d18f7051a47344d4d94e6143/airflow/contrib/task_runner/cgroup_task_runner.py#L172 command will fail. I have also looked into cgourpspy python package's source code and tests are few https://github.com/cloudsigma/cgroupspy/tree/master/cgroupspy. I am feeling that it is because cgroup relies on OS manipulation behavior so testing and mocking is hard? I am not able to test unittest for CgroupTaskRunner on my Mac because Mac does not support Cgroup. I was not able to test it in Docker either because Docker is already running in a cgroup so it will enter this condition https://github.com/apache/airflow/blob/9e72767b8e4de7f5d18f7051a47344d4d94e6143/airflow/contrib/task_runner/cgroup_task_runner.py#L121-#L129, which skips the core functionality for CgroupTaskRunner. How I verified this fix is working is that I pushed this change to our staging environment and directly tested it there, so it took me really a long time to get it work. See this original PR for CgroupTaskRunner: https://github.com/apache/airflow/pull/1934. The author just tested it in Airbnb Prod environment actually without writing any unittest. What comes to my mind is that we can test import errors in CgroupTaskRunner so if base_task_runner.py moves to another location again, the test can capture it. I know this does not sound ideal, but I don't know what I can do. I am actually willing to write tests if you can provide me some examples and suggestions.
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
