[GitHub] [airflow] feluelle commented on a change in pull request #5162: [AIRFLOW-4358] Speed up test_jobs by not running tasks
feluelle commented on a change in pull request #5162: [AIRFLOW-4358] Speed up
test_jobs by not running tasks
URL: https://github.com/apache/airflow/pull/5162#discussion_r279549347
##
File path: tests/executors/test_executor.py
##
@@ -16,47 +16,73 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+
+from collections import defaultdict
+
from airflow.executors.base_executor import BaseExecutor
from airflow.utils.state import State
-
-from airflow import settings
+from airflow.utils.db import create_session
class TestExecutor(BaseExecutor):
"""
TestExecutor is used for unit testing purposes.
"""
-def __init__(self, do_update=False, *args, **kwargs):
+def __init__(self, do_update=True, *args, **kwargs):
self.do_update = do_update
self._running = []
+
+# A list of "batches" of tasks
self.history = []
+# All the tasks, in a stable sort order
+self.sorted_tasks = []
+self.mock_task_results = defaultdict(lambda: State.SUCCESS)
super().__init__(*args, **kwargs)
-def execute_async(self, key, command, queue=None):
-self.log.debug("{} running task instances".format(len(self.running)))
-self.log.debug("{} in queue".format(len(self.queued_tasks)))
-
def heartbeat(self):
-session = settings.Session()
-if self.do_update:
+if not self.do_update:
Review comment:
>[...] that this is purely Test-specific class so we can implement it's
behaviour as we want rather than mock it.
That's actually true. Agreed. :) LGTM then.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] feluelle commented on a change in pull request #5162: [AIRFLOW-4358] Speed up test_jobs by not running tasks
feluelle commented on a change in pull request #5162: [AIRFLOW-4358] Speed up
test_jobs by not running tasks
URL: https://github.com/apache/airflow/pull/5162#discussion_r279549347
##
File path: tests/executors/test_executor.py
##
@@ -16,47 +16,73 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+
+from collections import defaultdict
+
from airflow.executors.base_executor import BaseExecutor
from airflow.utils.state import State
-
-from airflow import settings
+from airflow.utils.db import create_session
class TestExecutor(BaseExecutor):
"""
TestExecutor is used for unit testing purposes.
"""
-def __init__(self, do_update=False, *args, **kwargs):
+def __init__(self, do_update=True, *args, **kwargs):
self.do_update = do_update
self._running = []
+
+# A list of "batches" of tasks
self.history = []
+# All the tasks, in a stable sort order
+self.sorted_tasks = []
+self.mock_task_results = defaultdict(lambda: State.SUCCESS)
super().__init__(*args, **kwargs)
-def execute_async(self, key, command, queue=None):
-self.log.debug("{} running task instances".format(len(self.running)))
-self.log.debug("{} in queue".format(len(self.queued_tasks)))
-
def heartbeat(self):
-session = settings.Session()
-if self.do_update:
+if not self.do_update:
Review comment:
>[...] that this is purely Test-specific class so we can implement it's
behaviour as we want rather than mock it.
That's actually true. Agreed. :)
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] feluelle commented on a change in pull request #5162: [AIRFLOW-4358] Speed up test_jobs by not running tasks
feluelle commented on a change in pull request #5162: [AIRFLOW-4358] Speed up
test_jobs by not running tasks
URL: https://github.com/apache/airflow/pull/5162#discussion_r279466744
##
File path: tests/executors/test_executor.py
##
@@ -16,47 +16,73 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+
+from collections import defaultdict
+
from airflow.executors.base_executor import BaseExecutor
from airflow.utils.state import State
-
-from airflow import settings
+from airflow.utils.db import create_session
class TestExecutor(BaseExecutor):
"""
TestExecutor is used for unit testing purposes.
"""
-def __init__(self, do_update=False, *args, **kwargs):
+def __init__(self, do_update=True, *args, **kwargs):
self.do_update = do_update
self._running = []
+
+# A list of "batches" of tasks
self.history = []
+# All the tasks, in a stable sort order
+self.sorted_tasks = []
+self.mock_task_results = defaultdict(lambda: State.SUCCESS)
super().__init__(*args, **kwargs)
-def execute_async(self, key, command, queue=None):
-self.log.debug("{} running task instances".format(len(self.running)))
-self.log.debug("{} in queue".format(len(self.queued_tasks)))
-
def heartbeat(self):
-session = settings.Session()
-if self.do_update:
+if not self.do_update:
Review comment:
You have `do_update=False` and `do_update=True` (default). The `do_update`
flag only decides whether to run the `heartbeat` function or not (to speed up
tests). So why not remove that flag and use mocking to not to run the
`heartbeat` function.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] feluelle commented on a change in pull request #5162: [AIRFLOW-4358] Speed up test_jobs by not running tasks
feluelle commented on a change in pull request #5162: [AIRFLOW-4358] Speed up
test_jobs by not running tasks
URL: https://github.com/apache/airflow/pull/5162#discussion_r279185479
##
File path: tests/executors/test_executor.py
##
@@ -16,47 +16,73 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+
+from collections import defaultdict
+
from airflow.executors.base_executor import BaseExecutor
from airflow.utils.state import State
-
-from airflow import settings
+from airflow.utils.db import create_session
class TestExecutor(BaseExecutor):
"""
TestExecutor is used for unit testing purposes.
"""
-def __init__(self, do_update=False, *args, **kwargs):
+def __init__(self, do_update=True, *args, **kwargs):
self.do_update = do_update
self._running = []
+
+# A list of "batches" of tasks
self.history = []
+# All the tasks, in a stable sort order
+self.sorted_tasks = []
+self.mock_task_results = defaultdict(lambda: State.SUCCESS)
super().__init__(*args, **kwargs)
-def execute_async(self, key, command, queue=None):
-self.log.debug("{} running task instances".format(len(self.running)))
-self.log.debug("{} in queue".format(len(self.queued_tasks)))
-
def heartbeat(self):
-session = settings.Session()
-if self.do_update:
+if not self.do_update:
Review comment:
Don't you want to _mock_ the `heartbeat` here?
If so you could remove that extra statement and just patch the
`TestExecutor.heartbeat`
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:
us...@infra.apache.org
With regards,
Apache Git Services
