Repository: JonathonReinhart/scuba

Issue:
Cache YAML Files that are Referenced in !from_yaml Statements
## Problem
I am currently working on a large project that utilizes Scuba heavily. There are 27 scuba aliases in the `.scuba.yml` file, and all of the aliases have at least one `!from_yaml` directive in their definition. For example, many of the simple aliases look like this:
```yaml
product1_build:
  image: !from_yaml .gitlab-ci.yml product1_build.image
  script:
    - cd "$SCUBA_ROOT"
    - !from_yaml .gitlab-ci.yml product1_build.script
```

This means that there are 46 `!from_yaml` directives all referencing the same YAML file: `.gitlab-ci.yml`. Because Scuba re-parses this YAML file for every `!from_yaml`, this significantly slows down Scuba.

Tab completion is especially impacted by this; it can take almost 5 seconds for tab completion to complete. Since tab completion is mostly just a wrapper around `scuba --list-aliases`, we can use this command to benchmark tab completion:
```sh
$ time scuba --list-aliases
ALIAS             IMAGE
product1_build    debian:buster

real   0m4.784s
user   0m4.696s
sys    0m0.048s
```

## Suggested Solution
Scuba could cache files that have already been parsed in a previous `!from_yaml` invocation. This would eliminate the time needed to re-read and re-parse a file that has already been read and parsed.

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/JonathonReinhart/scuba/pull/149?src=pr&el=h1) Report
> Merging [#149](https://codecov.io/gh/JonathonReinhart/scuba/pull/149?src=pr&el=desc) into [master](https://codecov.io/gh/JonathonReinhart/scuba/commit/31e776732de59b856293e86358ab183a63e498b5?src=pr&el=desc) will **decrease** coverage by `0.77%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/JonathonReinhart/scuba/pull/149/graphs/tree.svg?width=650&token=EHDjbhs3va&height=150&src=pr)](https://codecov.io/gh/JonathonReinhart/scuba/pull/149?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #149      +/-   ##
==========================================
- Coverage   94.96%   94.19%   -0.78%     
==========================================
  Files           9        9              
  Lines         616      620       +4     
==========================================
- Hits          585      584       -1     
- Misses         31       36       +5
```


| [Impacted Files](https://codecov.io/gh/JonathonReinhart/scuba/pull/149?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [scuba/config.py](https://codecov.io/gh/JonathonReinhart/scuba/pull/149/diff?src=pr&el=tree#diff-c2N1YmEvY29uZmlnLnB5) | `97.32% <100%> (-1.04%)` | :arrow_down: |
| [scuba/compat.py](https://codecov.io/gh/JonathonReinhart/scuba/pull/149/diff?src=pr&el=tree#diff-c2N1YmEvY29tcGF0LnB5) | `90.9% <0%> (-9.1%)` | :arrow_down: |
| [scuba/utils.py](https://codecov.io/gh/JonathonReinhart/scuba/pull/149/diff?src=pr&el=tree#diff-c2N1YmEvdXRpbHMucHk=) | `93.18% <0%> (-4.55%)` | :arrow_down: |
| [scuba/version.py](https://codecov.io/gh/JonathonReinhart/scuba/pull/149/diff?src=pr&el=tree#diff-c2N1YmEvdmVyc2lvbi5weQ==) | `68.42% <0%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/JonathonReinhart/scuba/pull/149?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/JonathonReinhart/scuba/pull/149?src=pr&el=footer). Last update [31e7767...3a120bc](https://codecov.io/gh/JonathonReinhart/scuba/pull/149?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 11c03c0..9bced9b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,8 @@ All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
+### Added
+- Cache yaml files loaded by `!from_yaml`
 ### Removed
 - Drop support for Python 3.4
 
diff --git a/scuba/compat.py b/scuba/compat.py
index 12e739d..435aa36 100644
--- a/scuba/compat.py
+++ b/scuba/compat.py
@@ -1,7 +1,9 @@
 # Python 2/3 compatibility
 import subprocess
+import sys
 
 __all__ = [
+    'builtins_module_name',
     'File',
     'StringIO',
 ]
@@ -28,3 +30,10 @@ except NameError:
     # 'file' type removed, but open() returns _io.TextIOWrapper
     # which has a __dict__
     File = open
+
+
+# https://stackoverflow.com/a/9047762
+if sys.version_info >= (3,):
+    builtins_module_name = 'builtins'
+else:
+    builtins_module_name = '__builtin__'
diff --git a/scuba/config.py b/scuba/config.py
index e662807..8cea3ab 100644
--- a/scuba/config.py
+++ b/scuba/config.py
@@ -21,6 +21,7 @@ class ConfigNotFoundError(ConfigError):
 class Loader(yaml.SafeLoader):
     def __init__(self, stream):
         self._root = os.path.split(stream.name)[0]
+        self._cache = dict()
         super(Loader, self).__init__(stream)
 
     def from_yaml(self, node):
@@ -57,8 +58,11 @@ class Loader(yaml.SafeLoader):
         path = os.path.join(self._root, filename)
 
         # Load the other YAML document
-        with open(path, 'r') as f:
-            doc = yaml.load(f, self.__class__)
+        doc = self._cache.get(path)
+        if not doc:
+            with open(path, 'r') as f:
+                doc = yaml.load(f, self.__class__)
+                self._cache[path] = doc
 
         # Retrieve the key
         try:
@@ -328,7 +332,7 @@ class ScubaConfig(object):
 
 def load_config(path):
     try:
-        with open(path) as f:
+        with open(path, 'r') as f:
             data = yaml.load(f, Loader)
     except IOError as e:
         raise ConfigError('Error opening {}: {}'.format(SCUBA_YML, e))


Test patch:
diff --git a/tests/test_config.py b/tests/test_config.py
index 4da2d2f..a53c3cf 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -191,6 +191,33 @@ class TestConfig(TmpDirTestCase):
         config = scuba.config.load_config('.scuba.yml')
         assert_equals(config.image, 'debian:8.2')
 
+    def test_load_config_from_yaml_cached_file(self):
+        '''load_config loads a config using !from_yaml from cached version'''
+        with open('.gitlab.yml', 'w') as f:
+            f.write('one: debian:8.2\n')
+            f.write('two: debian:9.3\n')
+            f.write('three: debian:10.1\n')
+
+        with open('.scuba.yml', 'w') as f:
+            f.write('image: !from_yaml .gitlab.yml one\n')
+            f.write('aliases:\n')
+            f.write('  two:\n')
+            f.write('    image:  !from_yaml .gitlab.yml two\n')
+            f.write('    script: ugh\n')
+            f.write('  three:\n')
+            f.write('    image:  !from_yaml .gitlab.yml three\n')
+            f.write('    script: ugh\n')
+
+
+        with mock_open() as m:
+            config = scuba.config.load_config('.scuba.yml')
+
+        # Assert that .gitlab.yml was only opened once
+        assert_equals(m.mock_calls, [
+            mock.call('.scuba.yml', 'r'),
+            mock.call('.gitlab.yml', 'r'),
+        ])
+
     def test_load_config_image_from_yaml_nested_key_missing(self):
         '''load_config raises ConfigError when !from_yaml references nonexistant key'''
         with open('.gitlab.yml', 'w') as f:
diff --git a/tests/utils.py b/tests/utils.py
index ff8705c..5621efe 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -6,6 +6,7 @@ import tempfile
 import shutil
 import unittest
 import logging
+from scuba.compat import builtins_module_name
 try:
     from unittest import mock
 except ImportError:
@@ -58,6 +59,15 @@ def mocked_os_env(**env):
     return mock.patch('os.getenv', side_effect=env.get)
 
 
+def mock_open():
+    real_open = open
+    def mocked_open(*args, **kwargs):
+        return real_open(*args, **kwargs)
+
+    name = builtins_module_name + '.open'
+    return mock.patch(name, side_effect=mocked_open)
+
+
 # http://stackoverflow.com/a/8389373/119527
 class PseudoTTY(object):
     def __init__(self, underlying):