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

Run jupyter lite with subprocess.run to not suppress stdout #55

Merged
merged 3 commits into from
Jun 17, 2022

Conversation

jasongrout
Copy link
Collaborator

check_output redirects stdout, so we don't see it when we do sphinx-build. We instead switch to using subprocess.run so that we can see the stdout output.

Suppressing stdout was causing errors in jupyter lite to not be displayed, which was very confusing.

A separate issue is that jupyter lite is not returning a nonzero exit code when there is an error. But that is a problem to solve in jupyter lite. Once it is solved, having the check=True means that the sphinx build will also raise an error.

@jasongrout
Copy link
Collaborator Author

With this, I bet we'll start seeing the errors in building the current jupyterlite-sphinx docs.

@jasongrout
Copy link
Collaborator Author

With this, I bet we'll start seeing the errors in building the current jupyterlite-sphinx docs.

Yep, now we see the error at the bottom of https://readthedocs.org/api/v2/build/17204236.txt

Since jupyter lite still returned a zero error code indicating success, though, the docs build did not fail.

Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great thanks! It's been super annoying to debug jupyterlite-sphinx builds without this. Let's see if #56 fixes the lite build.

It looks like there is a black check failure on the Python code

@martinRenou
Copy link
Member

Can you please rebase your PR? We should now not see error logs with #56 in

check_output redirects stdout, so we don't see it when we do sphinx-build. We instead switch to using subprocess.run so that we can see the stdout output.

Suppressing stdout was causing errors in jupyter lite to not be displayed, which was very confusing.

A separate issue is that jupyter lite is not returning a nonzero exit code when there is an error. But that is a problem to solve in jupyter lite. Once it is solved, having the check=True means that the sphinx build will also raise an error.
@jasongrout
Copy link
Collaborator Author

Can you please rebase your PR? We should now not see error logs with #56 in

Done!

@jasongrout
Copy link
Collaborator Author

New error:

piplite:patch
.  post_build:contents:contents:.
.  post_build:federated_extensions:patch
.  post_build:federated_extensions:copy:theme:@telamonian/theme-darcula
.  post_build:federated_extensions:copy:theme:@timkpaine/jupyterlab_miami_nights
.  post_build:mimetypes:patch
.  post_build:piplite:meta:Jinja2-3.0.3-py3-none-any.whl.meta.json
TaskError - taskid:post_build:piplite:meta:Jinja2-3.0.3-py3-none-any.whl.meta.json
PythonAction Error
Traceback (most recent call last):
  File "/home/docs/checkouts/readthedocs.org/user_builds/jupyterlite-sphinx/envs/55/lib/python3.7/site-packages/doit/action.py", line 437, in execute
    returned_value = self.py_callable(*self.args, **kwargs)
  File "/home/docs/checkouts/readthedocs.org/user_builds/jupyterlite-sphinx/envs/55/lib/python3.7/site-packages/jupyterlite/addons/piplite.py", line 254, in index_wheel
    name, version, release = get_wheel_fileinfo(whl_path)
  File "/home/docs/checkouts/readthedocs.org/user_builds/jupyterlite-sphinx/envs/55/lib/python3.7/site-packages/jupyterlite/addons/piplite.py", line 269, in get_wheel_fileinfo
    import pkginfo
ModuleNotFoundError: No module named 'pkginfo'
[LiteBuildApp] Exiting application: jupyter

@martinRenou
Copy link
Member

Yes, this might be due to the new jupyterlite release that just came out. I'll have a look.

@jtpio
Copy link
Member

jtpio commented Jun 17, 2022

Maybe installing jupyterlite[piplite] or jupyterlite[all] here will help:

'jupyterlite',

Since pkginfo is defined here as a dependency:

https://github.com/jupyterlite/jupyterlite/blob/d6b790aaef7d585db968a354a66ff129b0f8c350/py/jupyterlite/pyproject.toml#L75

@jasongrout
Copy link
Collaborator Author

Yay, tests pass and docs look great!

Back to adding this to ipywidgets, once this is released...

@martinRenou
Copy link
Member

Great thanks Jason!

Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@martinRenou martinRenou merged commit f700de5 into jupyterlite:main Jun 17, 2022
@martinRenou
Copy link
Member

@jasongrout 0.4.8 is released :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants