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

Fix example capture.py #171

Merged
merged 3 commits into from
Dec 21, 2023
Merged

Conversation

hartwork
Copy link
Contributor

@hartwork hartwork commented Nov 25, 2023

Hi, thanks for examples/capture.py! 🙏

While playing with it I discovered three key issues about it and felt like offering a pull request with fixes to give back. These issues were:

  • Unhandled exception OSError
  • Providing an environment lacking HOME

As a bonus I resolved a needless else to reduce nesting and make the structure a bit simpler. Review on commit level is probably most efficient.

What do you think?

Symptom was:
> # python3 ./examples/capture.py output.txt asciinema rec -c 'timeout 3 htop' output.cast
> Traceback (most recent call last):
>   File "[..]/examples/capture.py", line 39, in <module>
>     data = os.read(master_fd, 1024)
> OSError: [Errno 5] Input/output error
>
> # python3 --version
> Python 3.10.13
@hartwork hartwork changed the title Fix example "capture py" Fix example capture.py Nov 25, 2023
.. to reduce nesting to simplify the structure.
Symptom was:
> # python3 ./examples/capture.py output.txt asciinema rec -c 'timeout 3 htop' output.cast
> Traceback (most recent call last):
>   File "/usr/lib/python-exec/python3.10/asciinema", line 8, in <module>
>     sys.exit(main())
>   File "/usr/lib/python3.10/site-packages/asciinema/__main__.py", line 61, in main
>     cfg = config.load()
>   File "/usr/lib/python3.10/site-packages/asciinema/config.py", line 226, in load
>     config = Config(get_config_home(env), env)
>   File "/usr/lib/python3.10/site-packages/asciinema/config.py", line 216, in get_config_home
>     raise Exception(
> Exception: need $HOME or $XDG_CONFIG_HOME or $ASCIINEMA_CONFIG_HOME
@hartwork
Copy link
Contributor Author

@selectel I believe this is ready for merge now. What do you think?

1 similar comment
@hartwork
Copy link
Contributor Author

hartwork commented Dec 9, 2023

@selectel I believe this is ready for merge now. What do you think?

@superbobry superbobry merged commit eac6000 into selectel:master Dec 21, 2023
5 checks passed
@superbobry
Copy link
Collaborator

Thanks, Sebastian.

@selectel does not maintain this repo, sadly. It's just me checking in once in a while.

@hartwork
Copy link
Contributor Author

@superbobry thanks for the merge! Has it been considered to move/transfer the repository to your account?

@superbobry
Copy link
Collaborator

I very much want @selectel to relicense pyte under a more permissive license first, and then transfer to another org/account.

Ping @tnt-dev :)

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

Successfully merging this pull request may close these issues.

2 participants