Merge pull request #240 from ModellingWebLab/191-commit-cache

Swap call to repo to use enhanced repocache

Author: jonc125

.travis.yml

@@ -15,7 +15,7 @@ before_script:
 install:
   pip install -r requirements/test.txt
 script:
-  - pytest --cov=weblab --cov-config=weblab/.coveragerc weblab
+  - pytest --fail-on-template-vars --cov=weblab --cov-config=weblab/.coveragerc weblab
   - flake8 weblab
   - isort --verbose --check-only --diff --recursive weblab
 after_success:

weblab/config/settings/dev.py

@@ -80,3 +80,14 @@
     app.split('.')[0]: copy.deepcopy(local_logger_conf)
     for app in LOCAL_APPS
 })
+
+
+class InvalidStringShowWarning(str):
+    def __mod__(self, other):
+        import logging
+        logger = logging.getLogger(__name__)
+        logger.warning("In template, undefined variable or unknown value for: '%s'" % (other,))
+        return ""
+
+
+TEMPLATES[0]['OPTIONS']['string_if_invalid'] = InvalidStringShowWarning("%s")

weblab/entities/models.py

@@ -6,6 +6,7 @@
 from django.core.urlresolvers import reverse
 from django.core.validators import MinLengthValidator
 from django.db import models
+from django.utils.functional import cached_property
 from guardian.shortcuts import get_objects_for_user
 
 from core.filetypes import get_file_type
@@ -72,11 +73,12 @@ class Meta:
     def __str__(self):
         return self.name
 
-    @property
+    @cached_property
     def repo(self):
         """This entity's git repository wrapper.
 
-        Note that we do not cache this property as this can lead to too many open files.
+        Caching this property actually reduces the number of open files, but there's still a risk
+        of running out of file handles eventually.
         See also https://gitpython.readthedocs.io/en/stable/intro.html#leakage-of-system-resources
         """
         return Repository(self.repo_abs_path)
@@ -92,11 +94,17 @@ def repo_abs_path(self):
             self.author.get_storage_dir('repo'), '%ss' % self.entity_type, str(self.id)
         )
 
-    def nice_version(self, commit):
-        version = self.repo.get_name_for_commit(commit)
-        if len(version) > 20:
-            version = version[:8] + '...'
-        return version
+    def nice_version(self, sha_or_tag):
+        """
+        Returns tag/sha with ellipses
+
+        :param sha_or_tag: version sha or tag string
+        :return version_name: string with the sha_or_tag formatted
+        """
+        version_name = self.repocache.get_name_for_version(sha_or_tag)
+        if len(version_name) > 20:
+            version_name = version_name[:8] + '...'
+        return version_name
 
     def get_visibility_from_repo(self, commit):
         """
@@ -296,7 +304,7 @@ def get_version_json(self, commit, ns):
             'visibility': self.get_version_visibility(commit.sha, default=self.DEFAULT_VISIBILITY),
             'created': commit.timestamp,
             'name': self.name,
-            'version': self.repo.get_name_for_commit(commit.sha),  # TODO #191 use repocache instead
+            'version': self.repocache.get_name_for_version(commit.sha),
             'files': files,
             'commitMessage': commit.message,
             'numFiles': len(files),

weblab/entities/repository.py

@@ -138,19 +138,6 @@ def tag_dict(self):
             tags.setdefault(tag.commit.hexsha, []).append(tag)
         return tags
 
-    def get_name_for_commit(self, version):
-        """Get a human-friendly display name for the given version
-
-        :param version: Revision specification (sha, branch name, tag etc.)
-            or 'latest' to get latest revision
-        :return: tag for this commit, if any, or version if not
-        """
-        commit = self.get_commit(version)
-        for tag in self._repo.tags:
-            if tag.commit == commit._commit:
-                return tag.name
-        return version
-
     def hard_reset(self):
         """
         Reset the working tree

weblab/entities/templatetags/entities.py

@@ -116,28 +116,32 @@ def url_entity_diff_base(context, entity_type):
 @register.filter
 def name_of_model(experiment):
     model = experiment.model
-    model_version = model.repo.get_name_for_commit(experiment.model_version)
+    model_version = model.repocache.get_name_for_version(experiment.model_version)
     return '%s @ %s' % (model.name, model_version)
 
 
 @register.filter
 def name_of_protocol(experiment):
     protocol = experiment.protocol
-    protocol_version = protocol.repo.get_name_for_commit(experiment.protocol_version)
+    protocol_version = protocol.repocache.get_name_for_version(experiment.protocol_version)
     return '%s @ %s' % (protocol.name, protocol_version)
 
 
-def _url_friendly_label(entity, commit):
+def _url_friendly_label(entity, version):
     """
     Get URL-friendly version label for a commit
 
     :param entity: Entity the commit belongs to
-    :param commit: `git.Commit` object
+    :param version: CachedEntityVersion object
     """
-    last_tag = str(entity.repo.tag_dict.get(commit.sha, ['/'])[-1])
-    if '/' in last_tag or last_tag in ['new', 'latest']:
-        last_tag = commit.sha
-    return last_tag
+    tags = version.tags.last()
+    if tags is not None:
+        tag = tags.tag
+        if tag is None or tag in ['new', 'latest']:
+            return version.sha
+        else:
+            return tag
+    return version.sha
 
 
 @register.filter

weblab/entities/tests/test_models.py

@@ -97,6 +97,7 @@ def test_nice_version(self, model_with_version):
         assert model_with_version.nice_version(commit) == '%s...' % commit[:8]
 
         model_with_version.repo.tag('v1')
+        populate_entity_cache(model_with_version)
         assert model_with_version.nice_version(commit) == 'v1'
 
     def test_set_and_get_version_visibility(self, model_with_version):

weblab/entities/tests/test_repository.py

@@ -77,19 +77,6 @@ def test_tag(self, repo, repo_file, author):
         with pytest.raises(GitCommandError):
             repo.tag('v1')
 
-    def test_name_for_commit(self, repo, repo_file, author):
-        repo.add_file(repo_file)
-        commit = repo.commit('commit_message', author)
-
-        assert repo.get_name_for_commit(commit.sha) == commit.sha
-        assert repo.get_name_for_commit('latest') == 'latest'
-
-        repo.tag('v1')
-
-        assert repo.get_name_for_commit(commit.sha) == 'v1'
-        assert repo.get_name_for_commit('v1') == 'v1'
-        assert repo.get_name_for_commit('latest') == 'v1'
-
     def test_has_changes(self, repo, repo_file, author):
         assert not repo.has_changes
         repo.add_file(repo_file)

weblab/entities/tests/test_templatetags.py

@@ -2,6 +2,7 @@
 
 import entities.templatetags.entities as entity_tags
 from core import recipes
+from repocache.populate import populate_entity_cache
 
 
 def test_human_readable_bytes():
@@ -34,7 +35,7 @@ def test_file_type():
 @pytest.mark.django_db
 def test_model_urls(model_with_version):
     model = model_with_version
-    model_version = model.repo.latest_commit
+    model_version = model.repocache.latest_version
     context = {'current_namespace': 'entities'}
 
     assert entity_tags.ns_url(context, 'new', 'model') == '/entities/models/new'
@@ -64,7 +65,7 @@ def test_model_urls(model_with_version):
 @pytest.mark.django_db
 def test_protocol_urls(protocol_with_version):
     protocol = protocol_with_version
-    protocol_version = protocol.repo.latest_commit
+    protocol_version = protocol.repocache.latest_version
     context = {'current_namespace': 'entities'}
 
     assert entity_tags.ns_url(context, 'new', 'protocol') == '/entities/protocols/new'
@@ -98,6 +99,8 @@ def test_protocol_urls(protocol_with_version):
 def test_name_of_entity_linked_to_experiment(model_with_version, protocol_with_version):
     model_with_version.repo.tag('v1')
     protocol_with_version.repo.tag('v2')
+    populate_entity_cache(model_with_version)
+    populate_entity_cache(protocol_with_version)
 
     exp = recipes.experiment_version.make(
         status='SUCCESS',
@@ -114,29 +117,39 @@ def test_name_of_entity_linked_to_experiment(model_with_version, protocol_with_v
 @pytest.mark.django_db
 def test_url_friendly_label(model_with_version, helpers):
     commit = model_with_version.repo.latest_commit
-    assert entity_tags._url_friendly_label(model_with_version, commit) == commit.sha
+    version = model_with_version.repocache.get_version(commit.sha)
+
+    assert entity_tags._url_friendly_label(model_with_version, version) == commit.sha
 
     model_with_version.repo.tag('v1')
-    assert entity_tags._url_friendly_label(model_with_version, commit) == 'v1'
+    populate_entity_cache(model_with_version)
+
+    assert entity_tags._url_friendly_label(model_with_version, version) == 'v1'
 
     commit2 = helpers.add_version(model_with_version)
     model_with_version.repo.tag('new')
-    assert entity_tags._url_friendly_label(model_with_version, commit2) == commit2.sha
+    populate_entity_cache(model_with_version)
+    version2 = model_with_version.repocache.get_version(commit2.sha)
+
+    assert entity_tags._url_friendly_label(model_with_version, version2) == commit2.sha
 
     commit3 = helpers.add_version(model_with_version)
     model_with_version.repo.tag('latest')
-    assert entity_tags._url_friendly_label(model_with_version, commit3) == commit3.sha
+    populate_entity_cache(model_with_version)
+    version3 = model_with_version.repocache.get_version(commit3.sha)
+
+    assert entity_tags._url_friendly_label(model_with_version, version3) == commit3.sha
 
 
 @pytest.mark.django_db
 def test_url_runexperiments(model_with_version, protocol_with_version):
     model = model_with_version
-    model_commit = model.repo.latest_commit
+    model_commit = model.repocache.latest_version
     assert (entity_tags.url_run_experiments(model, model_commit) ==
             '/entities/models/%d/versions/%s/runexperiments' % (model.pk, model_commit.sha))
 
     protocol = protocol_with_version
-    protocol_commit = protocol.repo.latest_commit
+    protocol_commit = protocol.repocache.latest_version
     assert (entity_tags.url_run_experiments(protocol, protocol_commit) ==
             '/entities/protocols/%d/versions/%s/runexperiments' % (protocol.pk, protocol_commit.sha))