Skip to content

Commit

Permalink
Update timing data keys, not overwrite it entirely.
Browse files Browse the repository at this point in the history
This will be necessary when we implement overridden atoms from build
requests. In such a build request, we will not have ran all the atoms,
and if we were to keep the existing behavior, we would wipe out all of
the atom timing data except for the select few that were manually
specified in the build request.

This method also leaves the door open in the future if we want to always
update the timing data, regardless of overall build success/failure, but
only for successful atoms.
  • Loading branch information
TJ Lee committed Aug 20, 2015
1 parent 2a25f5a commit 9fd6d17
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 8 deletions.
24 changes: 19 additions & 5 deletions app/master/build_artifact.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ def write_timing_data(self, timing_file_path, timing_data):
self._logger.debug('Created new timing file in {}', timing_file_path)
return

# If file exists, only overwrite timing data if there were no failures this build
# If file exists, only update timing data if there were no failures this build
if len(self.get_failed_commands()) == 0:
self._write_timing_data_to_file(timing_file_path, timing_data)
self._update_timing_file(timing_file_path, timing_data)
self._logger.debug('Overwrote existing timing file in {}', timing_file_path)
return

Expand Down Expand Up @@ -104,12 +104,26 @@ def generate_failures_file(self):
with open(os.path.join(self.build_artifact_dir, 'failures.txt'), 'w') as f:
f.write("\n".join(failed_atoms))

def _write_timing_data_to_file(self, path, timing_data):
def _write_timing_data_to_file(self, timing_file_path, timing_data):
"""
:type path: str
:type timing_file_path: str
:type timing_data: dict[str, float]
"""
app.util.fs.write_file(json.dumps(timing_data), path)
app.util.fs.write_file(json.dumps(timing_data), timing_file_path)

def _update_timing_file(self, timing_file_path, new_timing_data):
"""
Update the timing data for the atoms specified in new_timing_data. This means that new results
does not replace the entire timing data file, but rather only replaces the timing data for
individual atom keys.
:param timing_file_path: str
:param new_timing_data: dict[str, float]
"""
with open(timing_file_path) as timing_file:
timing_data = json.load(timing_file)

timing_data.update(new_timing_data)
app.util.fs.write_file(json.dumps(timing_data), timing_file_path)

@staticmethod
def atom_artifact_directory(build_id, subjob_id, atom_id, result_root=None):
Expand Down
7 changes: 4 additions & 3 deletions test/unit/master/test_atomizer.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from unittest.mock import Mock
from app.master.atomizer import Atomizer, AtomizerError

from app.master.atomizer import Atomizer, AtomizerError
from app.project_type.project_type import ProjectType
from app.util.process_utils import get_environment_variable_setter_command
from test.framework.base_unit_test_case import BaseUnitTestCase

Expand All @@ -13,7 +14,7 @@

class TestAtomizer(BaseUnitTestCase):
def test_atomizer_returns_expected_atom_list(self):
mock_project = Mock()
mock_project = Mock(spec=ProjectType)
mock_project.execute_command_in_project.return_value = (_FAKE_ATOMIZER_COMMAND_OUTPUT, _SUCCESSFUL_EXIT_CODE)
mock_project.project_directory = '/tmp/test/directory'

Expand All @@ -31,7 +32,7 @@ def test_atomizer_returns_expected_atom_list(self):
mock_project.execute_command_in_project.assert_called_once_with(_FAKE_ATOMIZER_COMMAND)

def test_atomizer_raises_exception_when_atomize_command_fails(self):
mock_project = Mock()
mock_project = Mock(spec=ProjectType)
mock_project.execute_command_in_project.return_value = ('ERROR ERROR ERROR', _FAILING_EXIT_CODE)

atomizer = Atomizer([{'TEST_FILE': _FAKE_ATOMIZER_COMMAND}])
Expand Down

0 comments on commit 9fd6d17

Please sign in to comment.