Bring doltcli up to date with dolt 1.16.0 (#44)

* Mark expected failure of `test_working` * Add typing for `create_test_table` fixture * Add typing for `init_empty_test_repo` fixture * Add `ORDER BY` clause to make test reliable * `test_merge_conflict` has incorrect logic * Fix fixture to set up a doltdb * Remove unused mixin file * Add `track` argument to checkout * Add option to specify the branch to pull from * Fix unit tests by updating `Branch` type * Fix merge function message parsing Wrong parsing of merge output was causing test_merge_fast_forward to fail * Add fix for test_update_rows This will be removed once issue with dolt table import -u is fixed dolthub/dolt#6675 * Add commit message to merge dolt command Improve merge docstring This fixes test_dolt_log_merge_commit test as now merge message is correctly used * Remove unreachable code in merge This code was never reach as --commit flag was always set by default in dolt merge command * Improve use of commit message variable in tests * Add recommended dolt version in README * Add remote and all options to branch * Update dolt version to 0.16.0 * Remove temporary csv import fix as solved in dolt * Simplify _get_branches * Add tests for pull command * Add tests for _get_branches --------- Co-authored-by: crisp-snakey <b.dejong@lumicks.com> Co-authored-by: radao <a.radice@lumicks.com> Co-authored-by: Tommaso <tommaso.gritti@gmail.com>
dolthub · Oct 4, 2023 · 87974a2 · 87974a2
1 parent f4e7c4c
commit 87974a2
Show file tree

Hide file tree

Showing 7 changed files with 254 additions and 89 deletions.
diff --git a/README.md b/README.md
@@ -3,7 +3,7 @@
 This is a minimalist package intended for data engineering applications:
 
 - unzipped size ~100kb
-- one dependency -- Dolt bunary
+- one dependency -- Dolt binary
 - only changes when Dolt changes
 
 If you are a data scientist or are using Pandas there are three options:
@@ -24,7 +24,7 @@ questions regarding production use-cases.
 
 - clone repo
 - Python 3.6+ required
-- [Install Dolt binary](https://docs.dolthub.com/getting-started/installation)
+- [Install Dolt binary](https://docs.dolthub.com/getting-started/installation). Currently recommended version is [1.16.0](https://github.com/dolthub/dolt/releases/tag/v1.16.0).
 - [Install Poetry](https://python-poetry.org/docs/#installation)
 ```bash
 curl -sSL https://raw.githubusercontent.com/python-poetry/poetry/master/get-poetry.py | python -

diff --git a/doltcli/dolt.py b/doltcli/dolt.py
@@ -484,9 +484,11 @@ def merge(
         """
         Executes a merge operation. If conflicts result, the merge is aborted, as an interactive merge does not really
         make sense in a scripting environment, or at least we have not figured out how to model it in a way that does.
-        :param branch:
-        :param message:
-        :param squash:
+        :param branch: name of the branch to merge into the current branch
+        :param message: message to be used for the merge commit only in the case of an automatic
+            merge. In case of automatic merge without a message provided, the commit message will be
+            "Merge branch '<branch>' into '<current_branch>'"
+        :param squash: squash the commits from the merged branch into a single commit
         :return:
         """
         current_branch, branches = self._get_branches()
@@ -503,16 +505,19 @@ def merge(
 
         if squash:
             args.append("--squash")
-
+        if message:
+            args.extend(["--message", message])
         args.append(branch)
         output = self.execute(args, **kwargs).split("\n")
-        merge_conflict_pos = 2
 
-        if len(output) == 3 and "Fast-forward" in output[1]:
+        # TODO: this was and remains a hack, we need to parse the output properly
+        if len(output) > 1 and "Fast-forward" in output[0]:
             logger.info(f"Completed fast-forward merge of {branch} into {current_branch.name}")
             return
 
-        if len(output) == 5 and output[merge_conflict_pos].startswith("CONFLICT"):
+        # TODO: this was and remains a hack, we need to parse the output properly
+        merge_conflict_pos = 8
+        if len(output) > 1 and output[merge_conflict_pos].startswith("CONFLICT"):
             logger.warning(
                 f"""
                 The following merge conflict occurred merging {branch} to {current_branch.name}:
@@ -527,12 +532,6 @@ def merge(
         if message is None:
             message = f"Merged {current_branch.name} into {branch}"
         logger.info(message)
-        status = self.status()
-
-        for table in list(status.added_tables.keys()) + list(status.modified_tables.keys()):
-            self.add(table)
-
-        self.commit(message)
 
     def sql(
         self,
@@ -729,18 +728,35 @@ def branch(
         delete: bool = False,
         copy: bool = False,
         move: bool = False,
+        remote: bool = False,
+        all: bool = False,
         **kwargs,
     ):
         """
-        Checkout, create, delete, move, or copy, a branch. Only
-        :param branch_name:
-        :param start_point:
-        :param new_branch:
-        :param force:
-        :param delete:
-        :param copy:
-        :param move:
-        :return:
+        List, create, or delete branches.
+
+        If 'branch_name' is None, existing branches are listed, including remotely tracked branches
+        if 'remote' or 'all' are set. If 'branch_name' is provided, a new branch is created, checked
+        our, deleted, moved or copied.
+
+        :param branch_name: Name of branch to Checkout, create, delete, move, or copy.
+        :param start_point: A commit that a new branch should point at.
+        :param new_branch: Name of branch to copy to or rename to if 'copy' or 'move' is set.
+        :param force: Reset 'branch_name' to 'start_point', even if 'branch_name' exists already.
+            Without 'force', dolt branch refuses to change an existing branch. In combination with
+            'delete', allow deleting the branch irrespective of its merged status. In
+            combination with 'move', allow renaming the branch even if the new branch name
+            already exists, the same applies for 'copy'.
+        :param delete: Delete a branch. The branch must be fully merged in its upstream branch.
+        :param copy: Create a copy of a branch.
+        :param move: Move/rename a branch. If 'new_branch' does not exist, 'branch_name' will be
+            renamed to 'new_branch'. If 'new_branch' exists, 'force' must be used to force the
+            rename to happen.
+        :param remote: When in list mode, show only remote tracked branches, unless 'all' is true.
+            When with -d, delete a remote tracking branch.
+        :param all: When in list mode, shows both local and remote tracked branches
+
+        :return: active_branch, branches
         """
         switch_count = [el for el in [delete, copy, move] if el]
         if len(switch_count) > 1:
@@ -751,7 +767,7 @@ def branch(
                 raise ValueError(
                     "force is not valid without providing a new branch name, or copy, move, or delete being true"
                 )
-            return self._get_branches()
+            return self._get_branches(remote=remote, all=all)
 
         args = ["branch"]
         if force:
@@ -780,6 +796,8 @@ def execute_wrapper(command_args: List[str]):
             if not branch_name:
                 raise ValueError("must provide branch_name when deleting")
             args.extend(["--delete", branch_name])
+            if remote:
+                args.append("--remote")
             return execute_wrapper(args)
 
         if move:
@@ -797,25 +815,42 @@ def execute_wrapper(command_args: List[str]):
                 args.append(start_point)
             return execute_wrapper(args)
 
-        return self._get_branches()
+        return self._get_branches(remote=remote, all=all)
 
-    def _get_branches(self) -> Tuple[Branch, List[Branch]]:
-        dicts = read_rows_sql(self, sql="select * from dolt_branches")
-        branches = [Branch(**d) for d in dicts]
+    def _get_branches(self, remote: bool = False, all: bool = False) -> Tuple[Branch, List[Branch]]:
+        """
+        Gets the branches for this repository, optionally including remote branches, and optionally
+        including all.
+
+        :param remote: include remotely tracked branches. If all is false and remote is true, only
+            remotely track branches are returned. If all is true both local and remote are included.
+            Default is False
+        :param all: include both local and remotely tracked branches. Default is False
+        :return: active_branch, branches
+        """
+        local_dicts = read_rows_sql(self, sql="select * from dolt_branches")
+        dicts = []
+        if all:
+            dicts = local_dicts + read_rows_sql(self, sql="select * from dolt_remote_branches")
+        elif remote:
+            dicts = read_rows_sql(self, sql="select * from dolt_remote_branches")
+        else:
+            dicts = local_dicts
+
+        # find active_branch
         ab_dicts = read_rows_sql(
             self, "select * from dolt_branches where name = (select active_branch())"
         )
-
         if len(ab_dicts) != 1:
             raise ValueError(
                 "Ensure you have the latest version of Dolt installed, this is fixed as of 0.24.2"
             )
-
         active_branch = Branch(**ab_dicts[0])
-
         if not active_branch:
             raise DoltException("Failed to set active branch")
 
+        branches = [Branch(**d) for d in dicts]
+
         return active_branch, branches
 
     def checkout(
@@ -824,6 +859,7 @@ def checkout(
         tables: Optional[Union[str, List[str]]] = None,
         checkout_branch: bool = False,
         start_point: Optional[str] = None,
+        track: Optional[str] = None,
         **kwargs,
     ):
         """
@@ -833,6 +869,7 @@ def checkout(
         :param tables: table or tables to checkout
         :param checkout_branch: branch to checkout
         :param start_point: tip of new branch
+        :param track: the upstream branch to track
         :return:
         """
         if tables and branch:
@@ -849,6 +886,10 @@ def checkout(
         if tables:
             args.append(" ".join(to_list(tables)))
 
+        if track is not None:
+            args.append("--track")
+            args.append(track)
+
         self.execute(args, **kwargs)
 
     def remote(
@@ -929,13 +970,18 @@ def push(
         # just print the output
         self.execute(args, **kwargs)
 
-    def pull(self, remote: str = "origin", **kwargs):
+    def pull(self, remote: str = "origin", branch: Optional[str] = None, **kwargs):
         """
         Pull the latest changes from the specified remote.
-        :param remote:
+        :param remote: The remote to pull the changes from
+        :param branch: The branch on the remote to pull the changes from
         :return:
         """
-        self.execute(["pull", remote], **kwargs)
+        args = ["pull", remote]
+        if branch is not None:
+            args.append(branch)
+
+        self.execute(args, **kwargs)
 
     def fetch(
         self,
@@ -1227,7 +1273,6 @@ def _config_helper(
         get: bool = False,
         unset: bool = False,
     ) -> Dict[str, str]:
-
         switch_count = [el for el in [add, list, get, unset] if el]
         if len(switch_count) != 1:
             raise ValueError("Exactly one of add, list, get, unset must be True")

diff --git a/doltcli/misc_mixin.py b/doltcli/misc_mixin.py
diff --git a/doltcli/types.py b/doltcli/types.py
@@ -26,6 +26,8 @@ class BranchT(BaseDataclass):
     latest_committer_email: Optional[str] = None
     latest_commit_date: Optional[datetime.datetime] = None
     latest_commit_message: Optional[str] = None
+    remote: Optional[str] = None
+    branch: Optional[str] = None
 
 
 @dataclass

diff --git a/tests/conftest.py b/tests/conftest.py
@@ -71,12 +71,12 @@ def doltdb():
         db = Dolt.init(db_path)
         db.sql("create table  t1 (a bigint primary key, b bigint, c bigint)")
         db.sql("insert into t1 values (1,1,1), (2,2,2)")
-        db.sql("call dolt_add('t1')")
-        db.sql("call dolt_commit('-m', 'initialize t1')")
+        db.add("t1")
+        db.commit("initialize t1")
 
         db.sql("insert into t1 values (3,3,3)")
-        db.sql("call dolt_add('t1')")
-        db.sql("call dolt_commit('-m', 'edit t1')")
+        db.add("t1")
+        db.commit("initialize edit t1")
         yield db_path
     finally:
         if os.path.exists(db_path):
@@ -112,6 +112,16 @@ def init_empty_test_repo(tmpdir) -> Dolt:
 def init_other_empty_test_repo(tmpdir) -> Dolt:
     return _init_helper(tmpdir, "other")
 
+@pytest.fixture
+def tmpdir2(tmpdir):
+    return tmpdir.mkdir("tmpdir2")
+
+@pytest.fixture
+def empty_test_repo_with_remote(tmpdir, tmpdir2) -> Dolt:
+    repo = _init_helper(tmpdir)
+    repo.remote(add=True, name="origin", url=rf"file:///{tmpdir2}")
+    return repo
+
 
 def _init_helper(path: str, ext: str = None):
     repo_path, repo_data_dir = get_repo_path_tmp_path(path, ext)