cli: add inspect subcommand #2184

kyrofa · 2018-07-30T21:52:16Z

Have you followed the guidelines for contributing?
Have you signed the CLA?
If this is a bugfix. Have you checked that there is a bug report open for the issue you are trying to fix on bug reports?
If this is a new feature. Have you discussed the design on the forum?
Have you successfully run ./runtests.sh static?
Have you successfully run ./runtests.sh unit?

There are three features that would be useful to snapcraft CLI developers that are not supported today:

A quick-look summary of the current project state.
A way to determine which lifecycle step of which part was most recently completed.
A way to determine which part provided a specific file within the staging or priming area.

This PR resolves LP: #1759700 by adding support for all three of these use-cases by introducing a new, hidden subcommand called inspect.

snapcraft inspect will print a quick summary of the current project state.
snapcraft inspect --latest-step will print the part, step, and timestamp of the latest step that was run.
snapcraft inspect --provides <path> will print the part that provided the path.

Default to human-readable output, but also support a --json to print json for programmatic consumption.

There are three features that would be useful to snapcraft CLI developers that are not supported today: 1. A quick-look summary of the current project state. 2. A way to determine which lifecycle step of which part was most recently completed. 3. A way to determine which part provided a specific file within the staging or priming area. Add support for all three of these use-cases by introducing a new, hidden subcommand called `inspect`. 1. `snapcraft inspect` will print a quick summary of the current project state. 2. `snapcraft inspect --latest-step` will print the part, step, and timestamp of the latest step that was run. 3. `snapcraft inspect --provides <path>` will print the part that provided the path. Default to human-readable output, but also support a `--json` to print json for programmatic consumption. LP: #1759700 Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>

sergiusens

Code looks clean and readable, nice.

There seems to be a problem when running on a not yet run project

(snapcraft) ubuntu@snapcraft-xenial-dev:~/source/make-hello$ snapcraft inspect --latest-step
Sorry, Snapcraft ran into an error when trying to running through its
lifecycle that generated the following traceback:
Traceback (most recent call last):
  File "/home/ubuntu/venv/snapcraft/bin/snapcraft", line 9, in <module>
    load_entry_point('snapcraft', 'console_scripts', 'snapcraft')()
  File "/home/ubuntu/venv/snapcraft/lib/python3.5/site-packages/click/core.py", line 722, in __call__
    return self.main(*args, **kwargs)
  File "/home/ubuntu/venv/snapcraft/lib/python3.5/site-packages/click/core.py", line 697, in main
    rv = self.invoke(ctx)
  File "/home/ubuntu/venv/snapcraft/lib/python3.5/site-packages/click/core.py", line 1066, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/ubuntu/venv/snapcraft/lib/python3.5/site-packages/click/core.py", line 895, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/ubuntu/venv/snapcraft/lib/python3.5/site-packages/click/core.py", line 535, in invoke
    return callback(*args, **kwargs)
  File "/home/ubuntu/source/snapcraft/snapcraft/cli/parts.py", line 157, in inspect
    step.name,
AttributeError: 'NoneType' object has no attribute 'name'
(snapcraft) ubuntu@snapcraft-xenial-dev:~/source/make-hello$ ls
Makefile  make-hello_0.1_amd64.snap  snapcraft.yaml  test.c
(snapcraft) ubuntu@snapcraft-xenial-dev:~/source/make-hello$ snapcraft pull
Pulling make-project 
(snapcraft) ubuntu@snapcraft-xenial-dev:~/source/make-hello$ snapcraft inspect --latest-step
The latest step that was run is the 'pull' step of the 'make-project' part, which ran at Mon Jul 30 23:02:34 2018
(snapcraft) ubuntu@snapcraft-xenial-dev:~/source/make-hello$ snapcraft inspect --latest-step --json
{
    "part": "make-project",
    "step": "pull",
    "timestamp": 1532991754.221821
}

For the json output at least we would need to get the working directory for that step too.

Great work so far

sergiusens · 2018-07-30T23:07:14Z

snapcraft/cli/parts.py

+        if option:
+            count += 1
+
+    if count > 1:


I wonder if it is worth expanding click like in this last comment on this issue pallets/click#257
Totally not a requirement for this PR, just a thought.

I think this is simple enough not to warrant more code, but if we find we need this functionality elsewhere, it may be worth doing.

Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>

kyrofa · 2018-07-31T15:53:49Z

There seems to be a problem when running on a not yet run project

Whoops, sorry, that was silly. Fixed.

For the json output at least we would need to get the working directory for that step too.

Sure, also fixed.

codecov-io · 2018-07-31T20:58:36Z

Codecov Report

Merging #2184 into master will increase coverage by 0.02%.
The diff coverage is 89.59%.

@@            Coverage Diff             @@
##           master    #2184      +/-   ##
==========================================
+ Coverage   91.19%   91.21%   +0.02%     
==========================================
  Files         202      208       +6     
  Lines       12992    13160     +168     
  Branches     1922     1957      +35     
==========================================
+ Hits        11848    12004     +156     
- Misses        776      788      +12     
  Partials      368      368

Impacted Files	Coverage Δ
...aft/internal/project_loader/inspection/__init__.py	`100% <100%> (ø)`
...internal/project_loader/inspection/_latest_step.py	`100% <100%> (ø)`
snapcraft/internal/lifecycle/__init__.py	`100% <100%> (ø)`	⬆️
snapcraft/cli/_runner.py	`95.34% <100%> (+0.11%)`	⬆️
...nal/project_loader/inspection/_lifecycle_status.py	`100% <100%> (ø)`
snapcraft/internal/states/__init__.py	`100% <100%> (ø)`	⬆️
snapcraft/cli/_command_group.py	`96.15% <100%> (+0.15%)`	⬆️
snapcraft/cli/parts.py	`82.14% <20%> (-13.51%)`	⬇️
snapcraft/internal/pluginhandler/__init__.py	`90.16% <52.94%> (-0.92%)`	⬇️
snapcraft/cli/inspect.py	`93.75% <93.75%> (ø)`
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5498d0a...da1d232. Read the comment docs.

Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>

…_inspect_subcommand

sergiusens · 2018-08-09T19:46:38Z

snapcraft/internal/errors.py

@@ -185,6 +185,18 @@ class SnapcraftPartMissingError(SnapcraftError):
    )


+class NoSuchFileError(SnapcraftError):


the error message seems to specific for this class' name. How about NoSuchFileInPartError.
These messages as with ToolError are getting a bit too un-namespaced btw. We should fix that.

sorry, but I just have to https://twitter.com/JenMsft/status/1027018324037623810

sergiusens · 2018-08-09T19:46:59Z

snapcraft/internal/errors.py

+        super().__init__(path=path)
+
+
+class UntrackedFileError(SnapcraftInspectError):


this is specific to parts too

sergiusens · 2018-08-09T19:48:10Z

snapcraft/internal/errors.py

+        super().__init__(path=path)
+
+
+class NoStepsRunError(SnapcraftInspectError):


ok, I see the pattern, we should prepend SnapcraftInspect to the class names (or put them in their own inspect package with most of the logic in the cli

sergiusens · 2018-08-09T19:48:49Z

snapcraft/internal/pluginhandler/__init__.py

@@ -126,24 +126,27 @@ def __init__(

    def get_pull_state(self) -> states.PullState:
        if not self._pull_state:
-            self._pull_state = states.get_state(self.plugin.statedir, steps.PULL)
+            self._pull_state = cast(states.PullState, self.get_state(steps.PULL))


why is this necessary?

Because this PR adds static checks to the return type of get_state. This now means casting if it's a child class.

kyrofa · 2018-08-10T16:21:20Z

snapcraft/cli/parts.py

+    return (latest_part, latest_step, latest_timestamp)
+
+
+def _lifecycle_status(config):


Move to package in project_loader and get rid of config.

Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>

…_inspect_subcommand Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>

Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>

…_inspect_subcommand

kyrofa mentioned this pull request Jul 30, 2018

cli: add provides command #2121

Closed

6 tasks

sergiusens requested changes Jul 30, 2018

View reviewed changes

sergiusens reviewed Jul 30, 2018

View reviewed changes

kyrofa added 2 commits July 31, 2018 08:37

inspect: --latest-step should handle no steps

5cb6e50

Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>

inspect: add working directory to output of --latest-step

81fff05

Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>

kyrofa added 2 commits August 2, 2018 10:27

Use different exit code for inspect errors

7dab431

Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>

Merge remote-tracking branch 'origin/master' into feature/1759700/add…

764a786

…_inspect_subcommand

sergiusens reviewed Aug 9, 2018

View reviewed changes

kyrofa commented Aug 10, 2018

View reviewed changes

Move inspection logic into project_loader

34a15c7

Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>

kyrofa force-pushed the feature/1759700/add_inspect_subcommand branch from 58385a5 to 34a15c7 Compare August 10, 2018 18:55

Merge remote-tracking branch 'origin/master' into feature/1759700/add…

0cea197

…_inspect_subcommand Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>

sergiusens approved these changes Aug 10, 2018

View reviewed changes

Reformat

ce88a59

Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>

kyrofa force-pushed the feature/1759700/add_inspect_subcommand branch from c178e4d to ce88a59 Compare August 11, 2018 14:20

kyrofa added 2 commits August 12, 2018 06:51

Fix setup.py

edc4404

Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>

Merge remote-tracking branch 'origin/master' into feature/1759700/add…

da1d232

…_inspect_subcommand

kyrofa merged commit 867df96 into canonical:master Aug 13, 2018

kyrofa deleted the feature/1759700/add_inspect_subcommand branch August 13, 2018 01:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cli: add inspect subcommand #2184

cli: add inspect subcommand #2184

kyrofa commented Jul 30, 2018 •

edited

Loading

sergiusens left a comment

sergiusens Jul 30, 2018

kyrofa Jul 31, 2018

kyrofa commented Jul 31, 2018

codecov-io commented Jul 31, 2018 •

edited

Loading

sergiusens Aug 9, 2018

sergiusens Aug 9, 2018

sergiusens Aug 9, 2018

sergiusens Aug 9, 2018

sergiusens Aug 9, 2018

kyrofa Aug 9, 2018 •

edited

Loading

kyrofa Aug 10, 2018 •

edited

Loading

kyrofa Aug 10, 2018

		@@ -185,6 +185,18 @@ class SnapcraftPartMissingError(SnapcraftError):
		)


		class NoSuchFileError(SnapcraftError):

		super().__init__(path=path)


		class UntrackedFileError(SnapcraftInspectError):

		super().__init__(path=path)


		class NoStepsRunError(SnapcraftInspectError):

		return (latest_part, latest_step, latest_timestamp)


		def _lifecycle_status(config):

cli: add inspect subcommand #2184

cli: add inspect subcommand #2184

Conversation

kyrofa commented Jul 30, 2018 • edited Loading

sergiusens left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kyrofa commented Jul 31, 2018

codecov-io commented Jul 31, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kyrofa Aug 9, 2018 • edited Loading

Choose a reason for hiding this comment

kyrofa Aug 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kyrofa commented Jul 30, 2018 •

edited

Loading

codecov-io commented Jul 31, 2018 •

edited

Loading

kyrofa Aug 9, 2018 •

edited

Loading

kyrofa Aug 10, 2018 •

edited

Loading