Fixes from code review

Author: wch

Diff for: rsconnect/bundle.py

@@ -264,25 +264,25 @@ def discard_from_buffer(self, key: str):
             del self.data["files"][key]
         return self
 
-    def check_and_get_entrypoint(self) -> str:
+    def require_entrypoint(self) -> str:
         """
         If self.entrypoint is a string, return it; if it is None, raise an exception.
         """
         if self.entrypoint is None:
             raise RSConnectException("A valid entrypoint must be provided.")
         return self.entrypoint
 
-    @property
-    def flattened_data(self) -> dict[str, ManifestDataFile]:
+    def get_manifest_files(self) -> dict[str, ManifestDataFile]:
         new_data_files: dict[str, ManifestDataFile] = {}
         deploy_dir: str
 
-        entrypoint = self.check_and_get_entrypoint()
+        entrypoint = self.require_entrypoint()
         if self.deploy_dir is not None:
             deploy_dir = self.deploy_dir
         elif entrypoint is not None and isfile(entrypoint):
             deploy_dir = dirname(entrypoint)
         else:
+            # TODO: This branch might be an error case. Need to investigate.
             deploy_dir = entrypoint
 
         for path in self.data["files"]:
@@ -291,18 +291,17 @@ def flattened_data(self) -> dict[str, ManifestDataFile]:
             new_data_files[manifestPath] = self.data["files"][path]
         return new_data_files
 
-    @property
-    def flattened_buffer(self) -> dict[str, str]:
+    def get_manifest_files_from_buffer(self) -> dict[str, str]:
         new_buffer: dict[str, str] = {}
-
         deploy_dir: str
 
-        entrypoint = self.check_and_get_entrypoint()
+        entrypoint = self.require_entrypoint()
         if self.deploy_dir is not None:
             deploy_dir = self.deploy_dir
         elif entrypoint is not None and isfile(entrypoint):
             deploy_dir = dirname(entrypoint)
         else:
+            # TODO: This branch might be an error case. Need to investigate.
             deploy_dir = entrypoint
 
         for k, v in self.buffer.items():
@@ -311,26 +310,22 @@ def flattened_buffer(self) -> dict[str, str]:
             new_buffer[manifestPath] = v
         return new_buffer
 
-    @property
-    def flattened_entrypoint(self) -> str:
-        entrypoint = self.check_and_get_entrypoint()
-        return relpath(entrypoint, dirname(entrypoint))
+    def get_relative_entrypoint(self) -> str:
+        entrypoint = self.require_entrypoint()
+        return basename(entrypoint)
 
-    @property
-    def flattened_primary_html(self):
+    def get_flattened_primary_html(self):
         if self.primary_html is None:
             raise RSConnectException("A valid primary_html must be provided.")
         return relpath(self.primary_html, dirname(self.primary_html))
 
-    @property
-    def flattened_copy(self):
-        self.check_and_get_entrypoint()
+    def get_flattened_copy(self):
         new_manifest = deepcopy(self)
-        new_manifest.data["files"] = self.flattened_data
-        new_manifest.buffer = self.flattened_buffer
-        new_manifest.entrypoint = self.flattened_entrypoint
+        new_manifest.data["files"] = self.get_manifest_files()
+        new_manifest.buffer = self.get_manifest_files_from_buffer()
+        new_manifest.entrypoint = self.get_relative_entrypoint()
         if self.primary_html:
-            new_manifest.primary_html = self.flattened_primary_html
+            new_manifest.primary_html = self.get_flattened_primary_html()
         return new_manifest
 
 
@@ -1087,10 +1082,10 @@ def make_html_bundle(
         if f in manifest.buffer:
             continue
         bundle.add_file(f)
-    for k, v in manifest.flattened_buffer.items():
+    for k, v in manifest.get_manifest_files_from_buffer().items():
         bundle.add_to_buffer(k, v)
 
-    manifest_flattened_copy_data = manifest.flattened_copy.data
+    manifest_flattened_copy_data = manifest.get_flattened_copy().data
     bundle.add_to_buffer("manifest.json", json.dumps(manifest_flattened_copy_data, indent=2))
 
     return bundle.to_file(manifest.deploy_dir)
@@ -1281,10 +1276,10 @@ def make_voila_bundle(
         if f in manifest.buffer:
             continue
         bundle.add_file(f)
-    for k, v in manifest.flattened_buffer.items():
+    for k, v in manifest.get_manifest_files_from_buffer().items():
         bundle.add_to_buffer(k, v)
 
-    manifest_flattened_copy_data = manifest.flattened_copy.data
+    manifest_flattened_copy_data = manifest.get_flattened_copy().data
     if multi_notebook and "metadata" in manifest_flattened_copy_data:
         manifest_flattened_copy_data["metadata"]["entrypoint"] = ""
     bundle.add_to_buffer("manifest.json", json.dumps(manifest_flattened_copy_data, indent=2))
@@ -1993,7 +1988,7 @@ def write_voila_manifest_json(
         raise RSConnectException("Voila manifest requires an entrypoint.")
 
     deploy_dir = dirname(manifest.entrypoint) if isfile(manifest.entrypoint) else manifest.entrypoint
-    manifest_flattened_copy_data = manifest.flattened_copy.data
+    manifest_flattened_copy_data = manifest.get_flattened_copy().data
     if multi_notebook and "metadata" in manifest_flattened_copy_data:
         manifest_flattened_copy_data["metadata"]["entrypoint"] = ""
     manifest_path = join(deploy_dir, "manifest.json")

Diff for: tests/test_Manifest.py

@@ -81,7 +81,7 @@ def test_Manifest_flattened_copy():
             "test_folder1/testfoldertext1.txt": {"checksum": "0a576fd324b6985bac6aa934131d2f5c"},
         },
     }
-    assert m.flattened_copy.data == html_manifest_dict
+    assert m.get_flattened_copy().data == html_manifest_dict
 
 
 def test_Manifest_empty_init():
@@ -97,4 +97,4 @@ def test_Manifest_empty_init():
 def test_Manifest_empty_exceptions():
     m = Manifest()
     with pytest.raises(RSConnectException) as _:
-        m.check_and_get_entrypoint()
+        m.require_entrypoint()

Diff for: tests/test_bundle.py

@@ -1463,7 +1463,7 @@ def test_create_voila_manifest_1(path, entrypoint):
             image=None,
             multi_notebook=False,
         )
-        assert ans == json.loads(manifest.flattened_copy.json)
+        assert ans == json.loads(manifest.get_flattened_copy().json)
 
 
 @pytest.mark.parametrize(
@@ -1521,7 +1521,7 @@ def test_create_voila_manifest_2(path, entrypoint):
         image=None,
         multi_notebook=False,
     )
-    assert ans == json.loads(manifest.flattened_copy.json)
+    assert ans == json.loads(manifest.get_flattened_copy().json)
 
 
 def test_create_voila_manifest_extra():
@@ -1569,7 +1569,7 @@ def test_create_voila_manifest_extra():
         image=None,
         multi_notebook=False,
     )
-    assert ans == json.loads(manifest.flattened_copy.json)
+    assert ans == json.loads(manifest.get_flattened_copy().json)
 
 
 @pytest.mark.parametrize(
@@ -1671,7 +1671,7 @@ def test_create_voila_manifest_multi_notebook(path, entrypoint):
             image=None,
             multi_notebook=True,
         )
-        assert json.loads(manifest.flattened_copy.json) == ans
+        assert json.loads(manifest.get_flattened_copy().json) == ans
 
 
 @pytest.mark.parametrize(
@@ -2077,7 +2077,7 @@ def test_create_html_manifest():
         extra_files=tuple(),
         excludes=tuple(),
     )
-    assert single_file_index_file_ans == json.loads(manifest.flattened_copy.json)
+    assert single_file_index_file_ans == json.loads(manifest.get_flattened_copy().json)
 
     # check all runtime_environment vars
     single_file_index_file_ans = {
@@ -2101,7 +2101,7 @@ def test_create_html_manifest():
         env_management_py=False,
         env_management_r=False,
     )
-    assert single_file_index_file_ans == json.loads(manifest.flattened_copy.json)
+    assert single_file_index_file_ans == json.loads(manifest.get_flattened_copy().json)
 
     # check image param
     single_file_index_file_ans = {
@@ -2121,7 +2121,7 @@ def test_create_html_manifest():
         env_management_py=None,
         env_management_r=None,
     )
-    assert single_file_index_file_ans == json.loads(manifest.flattened_copy.json)
+    assert single_file_index_file_ans == json.loads(manifest.get_flattened_copy().json)
 
     # check env_management_py param
     single_file_index_file_ans = {
@@ -2141,7 +2141,7 @@ def test_create_html_manifest():
         excludes=tuple(),
         env_management_py=False,
     )
-    assert single_file_index_file_ans == json.loads(manifest.flattened_copy.json)
+    assert single_file_index_file_ans == json.loads(manifest.get_flattened_copy().json)
 
     # check env_management_r param
     single_file_index_file_ans = {
@@ -2161,7 +2161,7 @@ def test_create_html_manifest():
         excludes=tuple(),
         env_management_r=False,
     )
-    assert single_file_index_file_ans == json.loads(manifest.flattened_copy.json)
+    assert single_file_index_file_ans == json.loads(manifest.get_flattened_copy().json)
 
     single_file_index_dir_ans = {
         "version": 1,
@@ -2179,15 +2179,15 @@ def test_create_html_manifest():
         extra_files=tuple(),
         excludes=tuple(),
     )
-    assert single_file_index_dir_ans == json.loads(manifest.flattened_copy.json)
+    assert single_file_index_dir_ans == json.loads(manifest.get_flattened_copy().json)
 
     manifest = create_html_manifest(
         single_file_index_dir,
         single_file_index_file,
         extra_files=tuple(),
         excludes=tuple(),
     )
-    assert single_file_index_dir_ans == json.loads(manifest.flattened_copy.json)
+    assert single_file_index_dir_ans == json.loads(manifest.get_flattened_copy().json)
 
     multi_file_index_file_ans = {
         "version": 1,
@@ -2201,7 +2201,7 @@ def test_create_html_manifest():
         extra_files=tuple(),
         excludes=tuple(),
     )
-    assert multi_file_index_file_ans == json.loads(manifest.flattened_copy.json)
+    assert multi_file_index_file_ans == json.loads(manifest.get_flattened_copy().json)
 
     multi_file_index_dir_ans = {
         "version": 1,
@@ -2218,15 +2218,15 @@ def test_create_html_manifest():
         extra_files=tuple(),
         excludes=tuple(),
     )
-    assert multi_file_index_dir_ans == json.loads(manifest.flattened_copy.json)
+    assert multi_file_index_dir_ans == json.loads(manifest.get_flattened_copy().json)
 
     manifest = create_html_manifest(
         multi_file_index_dir,
         multi_file_index_file,
         extra_files=tuple(),
         excludes=tuple(),
     )
-    assert multi_file_index_dir_ans == json.loads(manifest.flattened_copy.json)
+    assert multi_file_index_dir_ans == json.loads(manifest.get_flattened_copy().json)
 
     multi_file_nonindex_file_ans = {
         "version": 1,
@@ -2240,7 +2240,7 @@ def test_create_html_manifest():
         extra_files=tuple(),
         excludes=tuple(),
     )
-    assert multi_file_nonindex_file_ans == json.loads(manifest.flattened_copy.json)
+    assert multi_file_nonindex_file_ans == json.loads(manifest.get_flattened_copy().json)
 
     multi_file_nonindex_dir_and_file_ans = {
         "version": 1,
@@ -2257,7 +2257,7 @@ def test_create_html_manifest():
         extra_files=tuple(),
         excludes=tuple(),
     )
-    assert multi_file_nonindex_dir_and_file_ans == json.loads(manifest.flattened_copy.json)
+    assert multi_file_nonindex_dir_and_file_ans == json.loads(manifest.get_flattened_copy().json)
 
     multi_file_nonindex_file_extras_ans = {
         "version": 1,
@@ -2273,7 +2273,7 @@ def test_create_html_manifest():
         extra_files=[multi_file_nonindex_filea],
         excludes=tuple(),
     )
-    assert multi_file_nonindex_file_extras_ans == json.loads(manifest.flattened_copy.json)
+    assert multi_file_nonindex_file_extras_ans == json.loads(manifest.get_flattened_copy().json)
 
     multi_file_index_dir_extras_ans = {
         "version": 1,
@@ -2290,7 +2290,7 @@ def test_create_html_manifest():
         extra_files=[multi_file_index_file2],
         excludes=tuple(),
     )
-    assert multi_file_index_dir_extras_ans == json.loads(manifest.flattened_copy.json)
+    assert multi_file_index_dir_extras_ans == json.loads(manifest.get_flattened_copy().json)
 
 
 def test_make_html_bundle():