Repository: iterative/dvc

Issue:
safe `Dvcfile.dump()`
Initially, the idea was not to touch `dvc.yaml` at all, but as we are doing it quite often, it's better to protect `dvc.lock` rather than `dvc.yaml` by default.
So, we could change it to the following default:
```py
def dump(pipeline=True, lock=False):
    pass
```

The only place where we need to update `lock` are `dvc repro` and `dvc run`, at least atm.

_Originally posted by @skshetry in https://github.com/iterative/dvc/pull/3994#discussion_r437529134_

Discussion:

Patch:
diff --git a/dvc/config.py b/dvc/config.py
index 0f1bafac6..50bcfddc8 100644
--- a/dvc/config.py
+++ b/dvc/config.py
@@ -118,6 +118,8 @@ HTTP_COMMON = {
     "user": str,
     "password": str,
     "ask_password": Bool,
+    "ssl_verify": Bool,
+    "method": str,
 }
 WEBDAV_COMMON = {
     "user": str,
diff --git a/dvc/dvcfile.py b/dvc/dvcfile.py
index 6932a7bb7..972287362 100644
--- a/dvc/dvcfile.py
+++ b/dvc/dvcfile.py
@@ -175,7 +175,7 @@ class PipelineFile(FileMixin):
         return Lockfile(self.repo, os.path.splitext(self.path)[0] + ".lock")
 
     def dump(
-        self, stage, update_pipeline=False, no_lock=False, **kwargs
+        self, stage, update_pipeline=True, update_lock=True, **kwargs
     ):  # pylint: disable=arguments-differ
         """Dumps given stage appropriately in the dvcfile."""
         from dvc.stage import PipelineStage
@@ -187,7 +187,7 @@ class PipelineFile(FileMixin):
         if update_pipeline and not stage.is_data_source:
             self._dump_pipeline_file(stage)
 
-        if not no_lock:
+        if update_lock:
             self._dump_lockfile(stage)
 
     def _dump_lockfile(self, stage):
diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py
index dbc80b557..2822358f3 100644
--- a/dvc/repo/__init__.py
+++ b/dvc/repo/__init__.py
@@ -25,16 +25,30 @@ from .graph import check_acyclic, get_pipeline, get_pipelines
 logger = logging.getLogger(__name__)
 
 
+@contextmanager
+def lock_repo(repo):
+    # pylint: disable=protected-access
+    depth = getattr(repo, "_lock_depth", 0)
+    repo._lock_depth = depth + 1
+
+    try:
+        if depth > 0:
+            yield
+        else:
+            with repo.lock, repo.state:
+                repo._reset()
+                yield
+                # Graph cache is no longer valid after we release the repo.lock
+                repo._reset()
+    finally:
+        repo._lock_depth = depth
+
+
 def locked(f):
     @wraps(f)
     def wrapper(repo, *args, **kwargs):
-        with repo.lock, repo.state:
-            # pylint: disable=protected-access
-            repo._reset()
-            ret = f(repo, *args, **kwargs)
-            # Our graph cache is no longer valid after we release the repo.lock
-            repo._reset()
-            return ret
+        with lock_repo(repo):
+            return f(repo, *args, **kwargs)
 
     return wrapper
 
@@ -44,11 +58,11 @@ class Repo:
 
     from dvc.repo.add import add
     from dvc.repo.brancher import brancher
-    from dvc.repo.checkout import _checkout
+    from dvc.repo.checkout import checkout
     from dvc.repo.commit import commit
     from dvc.repo.destroy import destroy
     from dvc.repo.diff import diff
-    from dvc.repo.fetch import _fetch
+    from dvc.repo.fetch import fetch
     from dvc.repo.freeze import freeze, unfreeze
     from dvc.repo.gc import gc
     from dvc.repo.get import get
@@ -607,14 +621,6 @@ class Repo:
     def close(self):
         self.scm.close()
 
-    @locked
-    def checkout(self, *args, **kwargs):
-        return self._checkout(*args, **kwargs)
-
-    @locked
-    def fetch(self, *args, **kwargs):
-        return self._fetch(*args, **kwargs)
-
     def _reset(self):
         self.__dict__.pop("graph", None)
         self.__dict__.pop("stages", None)
diff --git a/dvc/repo/checkout.py b/dvc/repo/checkout.py
index bcfe31223..ce7238472 100644
--- a/dvc/repo/checkout.py
+++ b/dvc/repo/checkout.py
@@ -9,6 +9,8 @@ from dvc.exceptions import (
 from dvc.progress import Tqdm
 from dvc.utils import relpath
 
+from . import locked
+
 logger = logging.getLogger(__name__)
 
 
@@ -36,7 +38,8 @@ def get_all_files_numbers(pairs):
     )
 
 
-def _checkout(
+@locked
+def checkout(
     self,
     targets=None,
     with_deps=False,
diff --git a/dvc/repo/commit.py b/dvc/repo/commit.py
index e63ae59a4..331162c67 100644
--- a/dvc/repo/commit.py
+++ b/dvc/repo/commit.py
@@ -45,5 +45,5 @@ def commit(self, target, with_deps=False, recursive=False, force=False):
             stage.save()
         stage.commit()
 
-        Dvcfile(self, stage.path).dump(stage)
+        Dvcfile(self, stage.path).dump(stage, update_pipeline=False)
     return stages
diff --git a/dvc/repo/experiments/__init__.py b/dvc/repo/experiments/__init__.py
index ca5663f37..c57a24221 100644
--- a/dvc/repo/experiments/__init__.py
+++ b/dvc/repo/experiments/__init__.py
@@ -515,7 +515,7 @@ class Experiments:
         """Checkout an experiment to the user's workspace."""
         from git.exc import GitCommandError
 
-        from dvc.repo.checkout import _checkout as dvc_checkout
+        from dvc.repo.checkout import checkout as dvc_checkout
 
         self._check_baseline(rev)
         self._scm_checkout(rev)
diff --git a/dvc/repo/fetch.py b/dvc/repo/fetch.py
index 3fe807e07..1fe6546ca 100644
--- a/dvc/repo/fetch.py
+++ b/dvc/repo/fetch.py
@@ -4,10 +4,13 @@ from dvc.config import NoRemoteError
 from dvc.exceptions import DownloadError
 from dvc.scm.base import CloneError
 
+from . import locked
+
 logger = logging.getLogger(__name__)
 
 
-def _fetch(
+@locked
+def fetch(
     self,
     targets=None,
     jobs=None,
diff --git a/dvc/repo/freeze.py b/dvc/repo/freeze.py
index eb8500dc4..6776a06e4 100644
--- a/dvc/repo/freeze.py
+++ b/dvc/repo/freeze.py
@@ -8,7 +8,7 @@ def _set(repo, target, frozen):
     path, name = parse_target(target)
     stage = repo.get_stage(path, name)
     stage.frozen = frozen
-    stage.dvcfile.dump(stage, update_pipeline=True)
+    stage.dvcfile.dump(stage, update_lock=False)
 
     return stage
 
diff --git a/dvc/repo/plots/__init__.py b/dvc/repo/plots/__init__.py
index 4657d77f1..1d9165ea7 100644
--- a/dvc/repo/plots/__init__.py
+++ b/dvc/repo/plots/__init__.py
@@ -138,7 +138,7 @@ class Plots:
         out.verify_metric()
 
         dvcfile = Dvcfile(self.repo, out.stage.path)
-        dvcfile.dump(out.stage, update_pipeline=True, no_lock=True)
+        dvcfile.dump(out.stage, update_lock=False)
 
 
 def _collect_plots(repo, targets=None, rev=None):
diff --git a/dvc/repo/pull.py b/dvc/repo/pull.py
index 6e62652ca..a49d16938 100644
--- a/dvc/repo/pull.py
+++ b/dvc/repo/pull.py
@@ -22,7 +22,7 @@ def pull(
     if isinstance(targets, str):
         targets = [targets]
 
-    processed_files_count = self._fetch(  # pylint: disable=protected-access
+    processed_files_count = self.fetch(
         targets,
         jobs,
         remote=remote,
@@ -33,7 +33,7 @@ def pull(
         recursive=recursive,
         run_cache=run_cache,
     )
-    stats = self._checkout(  # pylint: disable=protected-access
+    stats = self.checkout(
         targets=targets, with_deps=with_deps, force=force, recursive=recursive
     )
 
diff --git a/dvc/repo/reproduce.py b/dvc/repo/reproduce.py
index 5b3be7b51..8fb665f87 100644
--- a/dvc/repo/reproduce.py
+++ b/dvc/repo/reproduce.py
@@ -25,7 +25,7 @@ def _reproduce_stage(stage, **kwargs):
         from ..dvcfile import Dvcfile
 
         dvcfile = Dvcfile(stage.repo, stage.path)
-        dvcfile.dump(stage)
+        dvcfile.dump(stage, update_pipeline=False)
 
     return [stage]
 
diff --git a/dvc/repo/run.py b/dvc/repo/run.py
index d383f4e25..c61c8142a 100644
--- a/dvc/repo/run.py
+++ b/dvc/repo/run.py
@@ -134,5 +134,5 @@ def run(self, fname=None, no_exec=False, single_stage=False, **kwargs):
             run_cache=kwargs.get("run_cache", True),
         )
 
-    dvcfile.dump(stage, update_pipeline=True, no_lock=no_exec)
+    dvcfile.dump(stage, update_lock=not no_exec)
     return stage
diff --git a/dvc/tree/dvc.py b/dvc/tree/dvc.py
index 6cbc1efe6..2fc8f8d6e 100644
--- a/dvc/tree/dvc.py
+++ b/dvc/tree/dvc.py
@@ -66,10 +66,6 @@ class DvcTree(BaseTree):  # pylint:disable=abstract-method
         except OutputNotFoundError as exc:
             raise FileNotFoundError from exc
 
-        # NOTE: this handles both dirty and checkout-ed out at the same time
-        if self.repo.tree.exists(path):
-            return self.repo.tree.open(path, mode=mode, encoding=encoding)
-
         if len(outs) != 1 or (
             outs[0].is_dir_checksum and path == outs[0].path_info
         ):
diff --git a/dvc/tree/http.py b/dvc/tree/http.py
index 18d793cf3..d084f66ee 100644
--- a/dvc/tree/http.py
+++ b/dvc/tree/http.py
@@ -53,6 +53,8 @@ class HTTPTree(BaseTree):  # pylint:disable=abstract-method
         self.password = config.get("password", None)
         self.ask_password = config.get("ask_password", False)
         self.headers = {}
+        self.ssl_verify = config.get("ssl_verify", True)
+        self.method = config.get("method", "POST")
 
     def _auth_method(self, path_info=None):
         from requests.auth import HTTPBasicAuth, HTTPDigestAuth
@@ -81,6 +83,8 @@ class HTTPTree(BaseTree):  # pylint:disable=abstract-method
 
         session = requests.Session()
 
+        session.verify = self.ssl_verify
+
         retries = Retry(
             total=self.SESSION_RETRIES,
             backoff_factor=self.SESSION_BACKOFF_FACTOR,
@@ -191,7 +195,7 @@ class HTTPTree(BaseTree):  # pylint:disable=abstract-method
                             break
                         yield chunk
 
-        response = self.request("POST", to_info.url, data=chunks())
+        response = self.request(self.method, to_info.url, data=chunks())
         if response.status_code not in (200, 201):
             raise HTTPError(response.status_code, response.reason)
 
diff --git a/dvc/tree/repo.py b/dvc/tree/repo.py
index 4861c068e..0b5620642 100644
--- a/dvc/tree/repo.py
+++ b/dvc/tree/repo.py
@@ -151,9 +151,13 @@ class RepoTree(BaseTree):  # pylint:disable=abstract-method
             encoding = None
 
         tree, dvc_tree = self._get_tree_pair(path)
-        if dvc_tree and dvc_tree.exists(path):
-            return dvc_tree.open(path, mode=mode, encoding=encoding, **kwargs)
-        return tree.open(path, mode=mode, encoding=encoding)
+        try:
+            return tree.open(path, mode=mode, encoding=encoding)
+        except FileNotFoundError:
+            if not dvc_tree:
+                raise
+
+        return dvc_tree.open(path, mode=mode, encoding=encoding, **kwargs)
 
     def exists(
         self, path, use_dvcignore=True


Test patch:
diff --git a/tests/func/test_dvcfile.py b/tests/func/test_dvcfile.py
index 2b029c20b..cccbc825b 100644
--- a/tests/func/test_dvcfile.py
+++ b/tests/func/test_dvcfile.py
@@ -242,7 +242,7 @@ def test_remove_stage_on_lockfile_format_error(tmp_dir, dvc, run_copy):
         dvc_file.remove_stage(stage)
 
     lock_file.remove()
-    dvc_file.dump(stage)
+    dvc_file.dump(stage, update_pipeline=False)
 
     dump_yaml(dvc_file.relpath, data)
     with pytest.raises(StageFileFormatError):
@@ -312,7 +312,7 @@ def test_dvcfile_dump_preserves_meta(tmp_dir, dvc, run_copy):
     data["stages"]["run_copy"]["meta"] = metadata
     dump_yaml(dvcfile.path, data)
 
-    dvcfile.dump(stage, update_pipeline=True)
+    dvcfile.dump(stage)
     assert dvcfile._load()[0] == data
     assert dvcfile._load()[0]["stages"]["run_copy"]["meta"] == metadata
 
@@ -332,5 +332,5 @@ def test_dvcfile_dump_preserves_comments(tmp_dir, dvc):
     stage.outs[0].use_cache = False
     dvcfile = stage.dvcfile
 
-    dvcfile.dump(stage, update_pipeline=True)
+    dvcfile.dump(stage)
     assert dvcfile._load()[1] == (text + ":\n\tcache: false\n".expandtabs())
diff --git a/tests/unit/remote/test_http.py b/tests/unit/remote/test_http.py
index 8a761a242..efb8716ba 100644
--- a/tests/unit/remote/test_http.py
+++ b/tests/unit/remote/test_http.py
@@ -80,3 +80,48 @@ def test_custom_auth_method(dvc):
     assert tree._auth_method() is None
     assert header in tree.headers
     assert tree.headers[header] == password
+
+
+def test_ssl_verify_is_enabled_by_default(dvc):
+    config = {
+        "url": "http://example.com/",
+        "path_info": "file.html",
+    }
+
+    tree = HTTPTree(dvc, config)
+
+    assert tree._session.verify is True
+
+
+def test_ssl_verify_disable(dvc):
+    config = {
+        "url": "http://example.com/",
+        "path_info": "file.html",
+        "ssl_verify": False,
+    }
+
+    tree = HTTPTree(dvc, config)
+
+    assert tree._session.verify is False
+
+
+def test_http_method(dvc):
+    from requests.auth import HTTPBasicAuth
+
+    user = "username"
+    password = "password"
+    auth = HTTPBasicAuth(user, password)
+    config = {
+        "url": "http://example.com/",
+        "path_info": "file.html",
+        "auth": "basic",
+        "user": user,
+        "password": password,
+        "method": "PUT",
+    }
+
+    tree = HTTPTree(dvc, config)
+
+    assert tree._auth_method() == auth
+    assert tree.method == "PUT"
+    assert isinstance(tree._auth_method(), HTTPBasicAuth)
diff --git a/tests/unit/repo/test_repo.py b/tests/unit/repo/test_repo.py
index 3a91e31bc..e63fe0d6b 100644
--- a/tests/unit/repo/test_repo.py
+++ b/tests/unit/repo/test_repo.py
@@ -60,9 +60,10 @@ def test_used_cache(tmp_dir, dvc, path):
 
 def test_locked(mocker):
     repo = mocker.MagicMock()
+    repo._lock_depth = 0
     repo.method = locked(repo.method)
 
-    args = {}
+    args = ()
     kwargs = {}
     repo.method(repo, args, kwargs)
 
diff --git a/tests/unit/test_dvcfile.py b/tests/unit/test_dvcfile.py
index abb736f8f..43b6bbcbd 100644
--- a/tests/unit/test_dvcfile.py
+++ b/tests/unit/test_dvcfile.py
@@ -13,6 +13,7 @@ from dvc.stage.exceptions import (
     StageFileFormatError,
     StageFileIsNotDvcFileError,
 )
+from dvc.utils.fs import remove
 from dvc.utils.serialize import dump_yaml
 
 
@@ -67,15 +68,18 @@ def test_dump_stage(tmp_dir, dvc):
     )
     dvcfile = Dvcfile(dvc, "dvc.yaml")
 
-    dvcfile.dump(stage, no_lock=True)
+    dvcfile.dump(stage, update_lock=False, update_pipeline=False)
     assert not (tmp_dir / PIPELINE_FILE).exists()
     assert not (tmp_dir / PIPELINE_LOCK).exists()
 
-    dvcfile.dump(stage, no_lock=False)
+    dvcfile.dump(stage, update_pipeline=False)
     assert not (tmp_dir / PIPELINE_FILE).exists()
+    assert (tmp_dir / PIPELINE_LOCK).exists()
     assert dvcfile._lockfile.load()
 
-    dvcfile.dump(stage, update_pipeline=True, no_lock=False)
+    remove(tmp_dir / PIPELINE_LOCK)
+
+    dvcfile.dump(stage)
     assert (tmp_dir / PIPELINE_FILE).exists()
     assert (tmp_dir / PIPELINE_LOCK).exists()
     assert list(dvcfile.stages.values()) == [stage]
diff --git a/tests/unit/tree/test_dvc.py b/tests/unit/tree/test_dvc.py
index 5f1fc33ea..af2c0bb5a 100644
--- a/tests/unit/tree/test_dvc.py
+++ b/tests/unit/tree/test_dvc.py
@@ -34,7 +34,9 @@ def test_open_dirty_hash(tmp_dir, dvc):
 
     tree = DvcTree(dvc)
     with tree.open("file", "r") as fobj:
-        assert fobj.read() == "something"
+        # NOTE: Unlike RepoTree, DvcTree should not
+        # be affected by a dirty workspace.
+        assert fobj.read() == "file"
 
 
 def test_open_dirty_no_hash(tmp_dir, dvc):
@@ -42,8 +44,11 @@ def test_open_dirty_no_hash(tmp_dir, dvc):
     (tmp_dir / "file.dvc").write_text("outs:\n- path: file\n")
 
     tree = DvcTree(dvc)
-    with tree.open("file", "r") as fobj:
-        assert fobj.read() == "file"
+    # NOTE: Unlike RepoTree, DvcTree should not
+    # be affected by a dirty workspace.
+    with pytest.raises(FileNotFoundError):
+        with tree.open("file", "r"):
+            pass
 
 
 def test_open_in_history(tmp_dir, scm, dvc):