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

pipeline: print locked DVC stages #1882

Merged
merged 9 commits into from Apr 16, 2019

Conversation

Projects
None yet
3 participants
@rpip
Copy link
Contributor

commented Apr 13, 2019

The --locked / -l option explicitly displays only locked DVC stages.

Fixes #1864

With the ‘—locked’ option:

$ dvc pipeline show --locked
Posts.tsv.dvc
$ dvc pipeline show --locked -c
python code/xml_to_tsv.py
$ dvc pipeline show --locked -o
data/Post.tsv

Without —locked option:

$ dvc pipeline show
data/Posts.xml.zip.dvc
Posts.xml.dvc
Posts.tsv.dvc
Posts-test.tsv.dvc
matrix-train.p.dvc
model.p.dvc
Dvcfile

@rpip rpip changed the title 1864 display locked stages pipeline: print locked DVC stages Apr 13, 2019

@efiop
Copy link
Member

left a comment

Looks great! 🎉 Travis tests are failing because of code formatting, which we have pre-commit hooks for that you need to install. Check this out https://dvc.org/doc/user-guide/contributing . It won't check already created commits, so you'll need to simply run checks by yourself like so:

$ black dvc/
$ black tests/
$ flake8 dvc/
$ flake8 tests/

Also, could you add a test for this, please?

@rpip

This comment has been minimized.

Copy link
Contributor Author

commented Apr 13, 2019

Fixed the code formatting. Will add the test for this.

self.dvc.add("bar")
self.dvc.lock_stage("foo.dvc")

ret = main(["pipeline", "show", "foo.dvc", "--locked"])

This comment has been minimized.

Copy link
@efiop

efiop Apr 15, 2019

Member

Sorry, our pipeline tests are quite lazy 🙂 Please use something like in tests/test_metrics.py as an example to make sure that the command outputs everything as expected. E.g. https://github.com/iterative/dvc/blob/0.35.7/tests/test_metrics.py#L227 .

@shcheklein
Copy link
Member

left a comment

Looks good! 👍Please update the documentation on dvc.org to include this new option.

rpip added some commits Apr 16, 2019

Merge branch 'master' into 1864-display-locked-stages
* master: (45 commits)
  s3: fix for corrupted cache on multipart uploads (#1867)
  Update PULL_REQUEST_TEMPLATE.md
  Create PULL_REQUEST_TEMPLATE.md
  Delete pull_request_template.md
  github: add PR template with a small checklist
  ignore: introduce .dvcignore, apply when searching for stage files
  add: dont use 'file' as var name during iteration
  test: move corrupted hardlink cache test test_repro -> test_run
  test_repro: make cache use hardlink
  cache: make reflink, copy default links
  diff: cli: add documentation link
  Update README.rst
  tests: be paranoic with caplog
  dvc: bump to 0.35.7
  dvc: deprecate --remove-outs
  logger: use exception instead of error
  version: show when the release version is dirty
  'till'=>'until' in `b_ref` arg. help text
  Adds space after period. (See previous commit.)
  Changes ',' for '.' in `-t` help text.
  ...
self.assertEqual(ret, 0)

expected_stdout = "foo.dvc"
assert expected_stdout in self._caplog.text

This comment has been minimized.

Copy link
@efiop

efiop Apr 16, 2019

Member

And what if pipeline show is going to print

foo.dvc
bar.dvc

? Your test would still pass, right? Looks like we need a more strict comparison 🙂

This comment has been minimized.

Copy link
@rpip

rpip Apr 16, 2019

Author Contributor

Yes. Wanted to do a more strict comparison but followed the convention.

Looking into how we can do a more strict comparison of command outputs. Both "\n".join(record.message for record in self._caplog.records) and self._caplog.text return the entire captures, which is verbose and pretty much defeats the purpose of testing for outputs.

This comment has been minimized.

Copy link
@efiop

efiop Apr 16, 2019

Member

@rpip Maybe something like:

assert "foo.dvc\n" in self._caplog.text
assert "bar.dvc\n" not in self._caplog.text

?

This comment has been minimized.

Copy link
@rpip

rpip Apr 16, 2019

Author Contributor

that won't work; debug logs from state.py#L164 contain "bar.dvc", and that shows in the captured log outputs.

self = <tests.test_pipeline.TestPipelineShow testMethod=test_print_locked_stages>

    def test_print_locked_stages(self):
        self._caplog.clear()

        self.dvc.add("foo")
        self.dvc.add("bar")
        self.dvc.lock_stage("foo.dvc")

        with self._caplog.at_level(logging.INFO, logger="dvc"):
            ret = main(["pipeline", "show", "foo.dvc", "--locked"])
            self.assertEqual(ret, 0)

        assert "foo.dvc" in self._caplog.text
>       assert "bar.dvc" not in self._caplog.text
E       AssertionError: assert 'bar.dvc' not in 'state.py             ... 37 INFO     foo.dvc\n'
E         'bar.dvc' is contained here:
E           state.py                   141 DEBUG    PRAGMA user_version;
E           state.py                   146 DEBUG    fetched: [(3,)]
E           state.py                   141 DEBUG    CREATE TABLE IF NOT EXISTS state (inode INTEGER PRIMARY KEY, mtime TEXT NOT NULL, size TEXT NO
T NULL, md5 TEXT NOT NULL, timestamp TEXT NOT NULL)
E           state.py                   141 DEBUG    CREATE TABLE IF NOT EXISTS state_info (count INTEGER)
E           state.py                   141 DEBUG    CREATE TABLE IF NOT EXISTS link_state (path TEXT PRIMARY KEY, inode INTEGER NOT NULL, mtime TE
XT NOT NULL)
E           state.py                   141 DEBUG    INSERT OR IGNORE INTO state_info (count) SELECT 0 WHERE NOT EXISTS (SELECT * FROM state_info)
E           state.py                   141 DEBUG    PRAGMA user_version = 3;
E           local.py                   780 DEBUG    Skipping copying for '/private/var/folders/gk/m8b9nwgj3n37ssb4mbn5h6dr0000gn/T/dvc-test.62418.
0fqdk2jh.a80c41ca-e87a-4008-9ba4-fda5b0c952d8/foo', since it is not a symlink or a hardlink.
E           fs.py                       19 DEBUG    Path /private/var/folders/gk/m8b9nwgj3n37ssb4mbn5h6dr0000gn/T/dvc-test.62418.0fqdk2jh.a80c41ca
-e87a-4008-9ba4-fda5b0c952d8/.dvc/cache/ac/bd18db4cc2f85cedef654fccc4a4d8 inode 8645937009
E           state.py                   141 DEBUG    SELECT mtime, size, md5, timestamp from state WHERE inode=8645937009
E           state.py                   146 DEBUG    fetched: [('1555416709490084864', '3', 'acbd18db4cc2f85cedef654fccc4a4d8', '155541671028488089
/Users/yao/dev/dvc: 0 4 1           1:ghc# 2:python3.6# 3:python2.7- 4:[tmux]* 5:bash# 6:bash# 7:bash# 8:bash# 9:bash#              | 16 Apr 12:12

This comment has been minimized.

Copy link
@rpip

rpip Apr 16, 2019

Author Contributor

tried debugging this and pretty printing the caplog records:

       ...
        import dvc.logger
        formatter = dvc.logger.ColorFormatter()
        print(formatter.format(caplog.records[0]))
        print(formatter.format(caplog.records[1]))
        print(formatter.format(caplog.records[...]))
       ....

nothing useful we can use for strict comparison atm. or am I missing something?

This comment has been minimized.

Copy link
@efiop

efiop Apr 16, 2019

Member

@rpip You just need to run self._caplog.clear() before with to cleanup old messages and then text should be as you expect it to be 🙂

This comment has been minimized.

Copy link
@rpip

rpip Apr 16, 2019

Author Contributor

ah yes, thanks. I called self._caplog.clear() too early. tried now, works -- pushing in a bit

This comment has been minimized.

Copy link
@rpip

rpip Apr 16, 2019

Author Contributor

@efiop fixed.

@efiop

efiop approved these changes Apr 16, 2019

@efiop

This comment has been minimized.

Copy link
Member

commented Apr 16, 2019

@rpip Thank you! 🚀

Btw, for the record: docs at iterative/dvc.org#250 (accidentally didn't get linked to this issue).

@efiop efiop merged commit 6c730dd into iterative:master Apr 16, 2019

3 checks passed

Travis CI - Pull Request Build Passed
Details
codeclimate All good!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.