crossbario / crossbar

Crossbar.io - WAMP application router
https://crossbar.io/
Other
2.05k stars 275 forks source link

Fix install from source #1884

Closed om26er closed 3 years ago

om26er commented 3 years ago

Crossbar was having dependency issues. This PR fixes that for all requirements-*.txt files.

This means crossbar can now be installed with default pip, with no special flags

om26er commented 3 years ago

This PR also fixes CI because it seems mypy 0.900 had a change that's causing https://github.com/crossbario/crossbar/pull/1884/checks?check_run_id=3197232792

This thread verifies that as well https://www.gitmemory.com/issue/python/mypy/10632/863332698

If we want to continue using the latest mypy then we need to change the mypy.ini to add an entry for each module that is producing those errors like

diff --git a/mypy.ini b/mypy.ini
index 61f1b9f6..762ea20a 100644
--- a/mypy.ini
+++ b/mypy.ini
@@ -3,3 +3,9 @@ python_version = 3.7

 [mypy-crossbar.worker.test.examples.*]
 ignore_errors = True
+
+[mypy-crossbar.*]
+ignore_missing_imports = True
+
+[mypy-six.*]
+ignore_missing_imports = True

How do we want to do it ?

om26er commented 3 years ago

@oberstet What are your thoughts about the CI fix, which approach would you like to take ?

Also does the installation fix look "acceptable" ?

oberstet commented 3 years ago

@om26er thanks a lot for digging, and for the change! rgd the mypy thing, I like the

+[mypy-crossbar.*]
+ignore_missing_imports = True
+
+[mypy-six.*]
+ignore_missing_imports = True

change .. as we can use latest mypy then. since the alternative (limit version to <0.900) also requires a change to the CI, but is more intrusive IMO ..

rgd the "installation fix": also looks good to me! the only thing: it'll be overwritten

https://github.com/crossbario/crossbar/blob/20e30b886698bb8416069c5e08d7814ab908a8a9/Makefile#L83

next time someone would use this make target to freeze and bump the pinned dependencies. unfortunately, these make targets are only "somewhat" automatic .. or put differently: even after so many iterations on CI, we are still not quite yet at the very sweet spot;)

anyways, thanks again! looks good

om26er commented 3 years ago

change .. as we can use latest mypy then. since the alternative (limit version to <0.900) also requires a change to the CI, but is more intrusive IMO ..

This is done now

om26er commented 3 years ago

Out of the new commits, this commit https://github.com/crossbario/crossbar/pull/1884/commits/85d44d2d59a802b358fff5b4ada2372ca2902cd1 was done to fix another mypy error.

(venv) om26er@HomePC:~/scm/crossbario/crossbar$ tox -c tox.ini -e mypy
mypy installed: crossbar==21.6.1.dev1,mypy==0.910,mypy-extensions==0.4.3,toml==0.10.2,typing-extensions==3.10.0.0
mypy run-test-pre: PYTHONHASHSEED='3260321038'
mypy run-test: commands[0] | mypy --exclude '(test_.*\.py)|(syntaxerror\.py)' --ignore-missing-imports crossbar
crossbar/network/_backend.py:1524: error: On Python 3 '{}'.format(b'abc') produces "b'abc'", not 'abc'; use '{!r}'.format(b'abc') if this is desired behavior
crossbar/network/_backend.py:1898: error: On Python 3 '{}'.format(b'abc') produces "b'abc'", not 'abc'; use '{!r}'.format(b'abc') if this is desired behavior
Found 2 errors in 1 file (checked 213 source files)
ERROR: InvocationError for command /home/om26er/scm/crossbario/crossbar/.tox/mypy/bin/mypy --exclude '(test_.*\.py)|(syntaxerror\.py)' --ignore-missing-imports crossbar (exited with code 1)
_______________________________________________________________________________ summary ________________________________________________________________________________
ERROR:   mypy: commands failed

The first error was correct, the second is a false alarm because the preceding line ensures contract_adr is NOT of type(bytes)

om26er commented 3 years ago

next time someone would use this make target to freeze and bump the pinned dependencies. unfortunately, these make targets are only "somewhat" automatic .. or put differently: even after so many iterations on CI, we are still not quite yet at the very sweet spot;)

We could probably add some sed in the Make file to take care of this stuff temporarily. Would that work ?

oberstet commented 3 years ago

so rgd the changes on this PR: this looks good to me! let me merge that .. the "sed" thing sounds good, if we can make it more bullet proof (as in, automate "bump versions and freeze"), but let's do that on a follow up PR (well, if you want to add / merge sth)

oberstet commented 3 years ago

fwiw, ci will be here https://github.com/crossbario/crossbar/actions/runs/1083690719