Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

exp push --rev broken #9471

Closed
dberenbaum opened this issue May 17, 2023 · 5 comments · Fixed by #9472 or #9483
Closed

exp push --rev broken #9471

dberenbaum opened this issue May 17, 2023 · 5 comments · Fixed by #9472 or #9483
Assignees
Labels
p1-important Important, aka current backlog of things to do regression Ohh, we broke something :-(

Comments

@dberenbaum
Copy link
Collaborator

Bug Report

Description

dvc exp push --rev is broken since #9391.

Reproduce

Run dvc exp push --rev HEAD from any repo and you will get:

$ dvc exp push -vv --rev HEAD origin 
2023-05-17 15:34:26,148 DEBUG: v2.55.0, CPython 3.10.10 on macOS-13.3.1-arm64-arm-64bit
2023-05-17 15:34:26,148 DEBUG: command: /Users/dave/miniforge3/envs/dvc/bin/dvc exp push -vv --rev HEAD origin
2023-05-17 15:34:26,148 TRACE: Namespace(cprofile=False, yappi=False, yappi_separate_threads=False, viztracer=False, viztracer_depth=None, viztracer_async=False, cprofile_dump=None, pdb=False, instrument=False, instrument_open=False, show_stack=False, quiet=0, verbose=2, cd='.', cmd='push', all_commits=False, rev=['HEAD'], num=1, force=False, push_cache=True, dvc_remote=None, jobs=None, run_cache=False, git_remote='origin', experiment=[], func=<class 'dvc.commands.experiments.push.CmdExperimentsPush'>, parser=DvcParser(prog='dvc', usage=None, description='Data Version Control', formatter_class=<class 'argparse.RawTextHelpFormatter'>, conflict_handler='error', add_help=False))
2023-05-17 15:34:26,370 ERROR: unexpected error - expected string or bytes-like object
Traceback (most recent call last):
  File "/Users/dave/Code/dvc/dvc/cli/__init__.py", line 210, in main
    ret = cmd.do_run()
  File "/Users/dave/Code/dvc/dvc/cli/command.py", line 26, in do_run
    return self.run()
  File "/Users/dave/Code/dvc/dvc/commands/experiments/push.py", line 65, in run
    result = self.repo.experiments.push(
  File "/Users/dave/Code/dvc/dvc/repo/experiments/__init__.py", line 494, in push
    return push(self.repo, *args, **kwargs)
  File "/Users/dave/Code/dvc/dvc/repo/__init__.py", line 65, in wrapper
    return f(repo, *args, **kwargs)
  File "/Users/dave/Code/dvc/dvc/repo/scm_context.py", line 151, in run
    return method(repo, *args, **kw)
  File "/Users/dave/Code/dvc/dvc/repo/experiments/push.py", line 115, in push
    exp_ref_set.update(exp_refs_from_rev(repo.scm, rev, num=num))
  File "/Users/dave/Code/dvc/dvc/repo/experiments/push.py", line 87, in exp_refs_from_rev
    rev_dict = iter_revs(scm, [rev], num)
  File "/Users/dave/Code/dvc/dvc/scm.py", line 266, in iter_revs
    return group_by(rev_resolver, results)
  File "/Users/dave/miniforge3/envs/dvc/lib/python3.10/site-packages/funcy/seqs.py", line 313, in group_by
    result[f(item)].append(item)
  File "/Users/dave/Code/dvc/dvc/scm.py", line 174, in resolve_rev
    return scm.resolve_rev(fix_exp_head(scm, rev))
  File "/Users/dave/Code/dvc/dvc/repo/experiments/utils.py", line 239, in fix_exp_head
    name, tail = Git.split_ref_pattern(ref)
  File "/Users/dave/miniforge3/envs/dvc/lib/python3.10/site-packages/scmrepo/git/__init__.py", line 157, in split_ref_pattern
    name = cls.BAD_REF_CHARS_RE.split(ref, maxsplit=1)[0]
TypeError: expected string or bytes-like object

2023-05-17 15:34:26,393 DEBUG: Removing '/Users/dave/Code/.CLFaTE3ctW8Fy6qXaisSEw.tmp'
2023-05-17 15:34:26,394 DEBUG: Removing '/Users/dave/Code/.CLFaTE3ctW8Fy6qXaisSEw.tmp'
2023-05-17 15:34:26,394 DEBUG: Removing '/Users/dave/Code/.CLFaTE3ctW8Fy6qXaisSEw.tmp'
2023-05-17 15:34:26,394 DEBUG: Removing '/Users/dave/Code/dvclive-exp-tracking/.dvc/cache/.YMaAXVC63teQCaVoXiTvho.tmp'
2023-05-17 15:34:26,395 DEBUG: Version info for developers:
DVC version: 2.55.0
-------------------
Platform: Python 3.10.10 on macOS-13.3.1-arm64-arm-64bit
Subprojects:
        dvc_data = 0.47.5
        dvc_objects = 0.22.0
        dvc_render = 0.3.1
        dvc_task = 0.2.1
        scmrepo = 1.0.3
Supports:
        azure (adlfs = 2023.1.0, knack = 0.10.1, azure-identity = 1.12.0),
        gdrive (pydrive2 = 1.15.3),
        gs (gcsfs = 2022.11.0),
        hdfs (fsspec = 2022.11.0, pyarrow = 11.0.0),
        http (aiohttp = 3.8.4, aiohttp-retry = 2.8.3),
        https (aiohttp = 3.8.4, aiohttp-retry = 2.8.3),
        oss (ossfs = 2021.8.0),
        s3 (s3fs = 2022.11.0, boto3 = 1.24.59),
        ssh (sshfs = 2023.4.1),
        webdav (webdav4 = 0.9.8),
        webdavs (webdav4 = 0.9.8),
        webhdfs (fsspec = 2022.11.0)
Config:
        Global: /Users/dave/Library/Application Support/dvc
        System: /Library/Application Support/dvc
Cache types: reflink, hardlink, symlink
Cache directory: apfs on /dev/disk3s1s1
Caches: local
Remotes: None
Workspace directory: apfs on /dev/disk3s1s1
Repo: dvc, git
Repo.site_cache_dir: /Library/Caches/dvc/repo/49450ff302024812849858c0b20a2692
@dberenbaum dberenbaum added regression Ohh, we broke something :-( p1-important Important, aka current backlog of things to do labels May 17, 2023
@daavoo daavoo self-assigned this May 17, 2023
@dberenbaum
Copy link
Collaborator Author

@daavoo Are you looking into it? I was going to take a look but don't want to duplicate the effort. We could either extend multiple rev support to all exp commands (looks like it impacts push, pull, remove, and list) or restrict this to exp show.

@daavoo
Copy link
Contributor

daavoo commented May 17, 2023

I have a fix, sending a patch

@daavoo daavoo linked a pull request May 17, 2023 that will close this issue
@daavoo
Copy link
Contributor

daavoo commented May 17, 2023

We could either extend multiple rev support to all exp commands (looks like it impacts push, pull, remove, and list) or restrict this to exp show.

Extending all those commands should be trivial. I thought they already supported multiple revs as the underlying iter_revs does.

Will send an update for the rest of the commands tomorrow

@dberenbaum
Copy link
Collaborator Author

Reopening until we have a patch for the other commands

@dberenbaum dberenbaum reopened this May 18, 2023
@dberenbaum dberenbaum added this to DVC May 18, 2023
@github-project-automation github-project-automation bot moved this to Backlog in DVC May 18, 2023
@dberenbaum dberenbaum moved this from Backlog to In Progress in DVC May 18, 2023
@dberenbaum
Copy link
Collaborator Author

@daavoo Are you still working on this one? I'd say it's almost a p0 since it breaks the --rev flag for multiple commands.

daavoo added a commit that referenced this issue May 21, 2023
daavoo added a commit that referenced this issue May 21, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in DVC May 21, 2023
daavoo added a commit that referenced this issue May 21, 2023
mergify bot pushed a commit that referenced this issue May 21, 2023
daavoo added a commit that referenced this issue May 21, 2023
Closes #9471

(cherry picked from commit 3ddd4b8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p1-important Important, aka current backlog of things to do regression Ohh, we broke something :-(
Projects
No open projects
Archived in project
2 participants