Repository: kedro-org/kedro
Pull request: [KED-1076] Add open flag to build-docs that opens the documentation on build
## Notice

- [x] I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

- I submit this contribution under the [Apache 2.0 license](https://www.apache.org/licenses/LICENSE-2.0.txt) and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
- I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorised to submit this contribution on behalf of the original creator(s) or their licensees.
- I certify that the use of this contribution as authorised by the Apache 2.0 license does not violate the intellectual property rights of anyone else.

## Motivation and Context
Why was this PR created?
Closes https://github.com/quantumblacklabs/kedro/issues/86

## How has this been tested?
What testing strategies have you used?
Similar testing strategy as `kedro.cli` with `docs`.

## Checklist

- [x] Read the [contributing](/CONTRIBUTING.md) guidelines
- [x] Opened this PR as a 'Draft Pull Request' if it is work-in-progress
- [ ] Updated the documentation to reflect the code changes
- [x] Added new entries to the `RELEASE.md` file
- [x] Added tests to cover my changes
- [x] Assigned myself to the PR


Problem statement:
[KED-1054] Add option to kedro build-docs which also opens the documentation
## Description
I find myself writing '`kedro build-docs ; open docs/build/html/index.html`', adding a simple 
`--open` argument would be appreciated. I do plan to PR this myself, I'm just not sure when I have time.

## Context
- Saves time
- Is almost always the next action after running the command

## Possible Implementation
- Add CLI argument

## Possible Alternatives
N/A

Hints from issue discussion:
Correct me if I'm wrong, but `open` is MacOS-specific, and `xdg-open` should be used on Linux.
That might be a cross-platform solution: https://docs.python.org/3.7/library/webbrowser.html

It would be amazing if you find some time to send a pull request. Thank you for raising this anyway!
@datajoely - as a temporary fix, I'd recommend looking at the `entr` CLI [tool](http://eradman.com/entrproject/).

You can use it like so:

```bash
ls *.py | entr -s 'kedro build-docs && open docs/build/html/index.html'
```

Effectively it would allow you to create a live reload server that watches for changes in Python source files and automatically serves the up-to-date documentation.
I thought this'd be a useful feature too! I built it for myself in my fork here: https://github.com/mzjp2/kedro/commit/468f328b5e774130fc367dea763ee6d3f3f79205

Happy to PR it (it's ready), if you want @datajoely, but equally also happy to let you if you want. :)
I've opened a PR, but feel free to close it in favour of Joel's if he makes one. :)

Commit message: Merge branch 'develop' into feature/open-docs-option

Patch to review:
diff --git a/RELEASE.md b/RELEASE.md
index 29f422e0cd..6baa6f08ff 100644
--- a/RELEASE.md
+++ b/RELEASE.md
@@ -11,12 +11,14 @@
 * Improved an error message when versioned local dataset is saved and unversioned path already exists.
 * Enabled tuples to be returned from a node.
 * Disallow the ConfigLoader loading the same file more than once, and deduplicate the `conf_paths` passed in
+* Added a `--open` flag to `kedro build-docs` that opens the documentation on build.
 
 ## Breaking changes to the API
 * `KedroContext.run()` no longer accepts `catalog` and `pipeline` arguments.
 
 ## Thanks for supporting contributions
-[Deepyaman Datta](https://github.com/deepyaman), [Luciano Issoe](https://github.com/Lucianois), [Joost Duisters](https://github.com/JoostDuisters)
+
+[Deepyaman Datta](https://github.com/deepyaman), [Luciano Issoe](https://github.com/Lucianois), [Joost Duisters](https://github.com/JoostDuisters), [Zain Patel](https://github.com/mzjp2)
 
 # Release 0.15.1
 
diff --git a/kedro/template/{{ cookiecutter.repo_name }}/kedro_cli.py b/kedro/template/{{ cookiecutter.repo_name }}/kedro_cli.py
index 4a15edafcb..733286c6fd 100755
--- a/kedro/template/{{ cookiecutter.repo_name }}/kedro_cli.py	
+++ b/kedro/template/{{ cookiecutter.repo_name }}/kedro_cli.py	
@@ -35,6 +35,7 @@
 from collections import Counter
 from glob import iglob
 from pathlib import Path
+import webbrowser
 
 import click
 from click import secho, style
@@ -92,6 +93,8 @@
 If not specified, use the `SequentialRunner`. This flag cannot be used together
 with --runner."""
 
+OPEN_ARG_HELP = """Open the documentation in your default browser after building."""
+
 RUNNER_ARG_HELP = """Specify a runner that you want to run the pipeline with.
 This option cannot be used together with --parallel."""
 
@@ -231,7 +234,8 @@ def package():
 
 
 @cli.command("build-docs")
-def build_docs():
+@click.option("--open", "-o", "open_docs", is_flag=True, multiple=False, default=False, help=OPEN_ARG_HELP)
+def build_docs(open_docs):
     """Build the project documentation."""
     python_call("pip", ["install", "src/[docs]"])
     python_call("pip", ["install", "-r", "src/requirements.txt"])
@@ -249,6 +253,11 @@ def build_docs():
         ]
     )
     call(["sphinx-build", "-M", "html", "docs/source", "docs/build", "-a"])
+    if open_docs:
+        p = Path("docs/build/html/index.html").resolve().as_uri()
+        secho("Opening {}".format(p))
+        webbrowser.open(str(p))
+
 
 
 @cli.command("build-reqs")
diff --git a/tests/template/test_kedro_cli.py b/tests/template/test_kedro_cli.py
index 7983ff0901..3fdf7da0d7 100644
--- a/tests/template/test_kedro_cli.py
+++ b/tests/template/test_kedro_cli.py
@@ -317,6 +317,20 @@ def test_happy_path(self, call_mock, python_call_mock, fake_kedro_cli, mocker):
         )
         fake_rmtree.assert_called_once_with("docs/build", ignore_errors=True)
 
+    @pytest.mark.parametrize("open_flag", ["-o", "--open"])
+    def test_open_docs(
+        self, open_flag, fake_repo_path, call_mock, python_call_mock, fake_kedro_cli, mocker
+    ):
+        docs_path = fake_repo_path / "docs" / "build" / "html" / "index.html"
+
+        # Python 3.5 pathlib.Path.resolve() has strict resolve (i.e: it checks the filepath exists)
+        mocker.patch.object(Path, "resolve", return_value=docs_path)
+        patched_browser = mocker.patch("webbrowser.open")
+
+        result = CliRunner().invoke(fake_kedro_cli.cli, ["build-docs", open_flag])
+        assert not result.exit_code, result.stdout
+        patched_browser.assert_called_once_with(str(docs_path.as_uri()))
+
 
 class TestBuildReqsCommand:
     def test_requirements_file_exists(self, python_call_mock, fake_kedro_cli, mocker):


Code review comments:
diff_hunk: @@ -249,6 +253,11 @@ def build_docs():
         ]
     )
     call(["sphinx-build", "-M", "html", "docs/source", "docs/build", "-a"])
+    if open_docs:
+        p = Path("docs/build/html/index.html").resolve().as_uri()
path: kedro/template/{{ cookiecutter.repo_name }}/kedro_cli.py
text: @user1:
Given the difference in path separators, I suspect this will not play nice with Windows. How about going for something like this instead:
```(Path.cwd() / "docs" / "build" / "html" / "index.html").as_uri()```
and let `pathlib` handle the separators?
This might also help you get rid of the mock object in your test, which is nice.

@author:
Ah, that's nice. Thanks, should be sorted now. :)

diff_hunk: @@ -249,6 +253,11 @@ def build_docs():
         ]
     )
     call(["sphinx-build", "-M", "html", "docs/source", "docs/build", "-a"])
+    if open_docs:
+        p = Path("docs/build/html/index.html").resolve().as_uri()
+        secho("Opening {}".format(p))
+        webbrowser.open(str(p))
path: kedro/template/{{ cookiecutter.repo_name }}/kedro_cli.py
text: @user1:
Can we replace "p" here with a more descriptive variable name? :) "docs_page", "docs_home" or anything like that would suffice.

@user1:
Also I think the return of `as_uri()` is a `string` anyway, so no need for casting here. If you want to be on the safe side regardless, you could add it to line 257, so that would get passed through to `format` and `open` calls.

@author:
Thanks for the feedback, super valuable! 🚀

Should be fixed now. :)

diff_hunk: @@ -317,6 +317,20 @@ def test_happy_path(self, call_mock, python_call_mock, fake_kedro_cli, mocker):
         )
         fake_rmtree.assert_called_once_with("docs/build", ignore_errors=True)
 
+    @pytest.mark.parametrize("open_flag", ["-o", "--open"])
+    def test_open_docs(
+        self, open_flag, fake_repo_path, call_mock, python_call_mock, fake_kedro_cli, mocker
path: tests/template/test_kedro_cli.py
text: @user1:
Since these 2 do not appear to be used below, can we safely remove them?
```suggestion
        self, open_flag, fake_repo_path, fake_kedro_cli, mocker
```

@author:
Removed those, my bad 🙈 (and also `fake_repo_path` since I don't need it anymore)! 🎉