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

Support poetry and fix websocket crash #64

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

Conversation

jcable
Copy link
Collaborator

@jcable jcable commented Sep 16, 2023

The README.md will need changing as it links to my fork.

pyproject.toml is the modern way to define python projects. Poetry is a good pyproject.toml based environment allowing different python versions to be selected and managing virtual environments.

I had to pin at python 3.9 as PyXB is incompatible with 3.10 and above.

The websocket sender crashed as soon as the web page connected but encoding the string to bytes fixed this.

@nigelmegitt
Copy link
Collaborator

Thanks for this @jcable - appreciate the extra eyes on here and the input!

What's the motivation for switching to poetry? I'm not against it per se, just not sure it actually fixes anything. I also see that travis CI is still on here, and we need to switch to GitHub Actions preferably.

For preference, I'd rather have more granular pull requests, e.g. the websocket fixes and python3.9 pinning as a separate pull request but it's a fussy point.

I'm curious about marking all the xfail tests as skip and the comment about mocking internet connections - is there a separate issue that needs raising there?

@nigelmegitt
Copy link
Collaborator

A lot of this has now been superseded by merging different PRs that updated to Python3.11 and added a poetry build (at least partially based on the work @jcable did here). Suggest refactoring (or closing in favour of a new pull request) to just deal with the websocket issues.

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