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 tests against Python 3.8 #93

Merged
merged 3 commits into from
Nov 13, 2019
Merged

Run tests against Python 3.8 #93

merged 3 commits into from
Nov 13, 2019

Conversation

michael-k
Copy link
Contributor

No description provided.

@michael-k
Copy link
Contributor Author

Despite setting PYTHON_VERSION: "3.8.0", appveyor used Python 2.7.
https://ci.appveyor.com/project/duyue/hiredis-py/builds/28350884/job/xbid8ymbjdt08r3y#L8

It looks like they don't support Python 3.8 yet: appveyor/ci#3142

@ifduyue
Copy link
Collaborator

ifduyue commented Oct 27, 2019

LGTM

I see there's a workaround from the link you provided above appveyor/ci#3142 (comment), would mind adding it?

@michael-k
Copy link
Contributor Author

I've added the workaround and now Python 3.8 is available:

python --version
Python 3.8.0

But now the tests are failing:

Executing: python test.py
Traceback (most recent call last):
  File "test.py", line 4, in <module>
    import test
  File "C:\projects\hiredis-py\test\__init__.py", line 11, in <module>
    from . import reader
  File "C:\projects\hiredis-py\test\reader.py", line 3, in <module>
    import hiredis
ImportError: DLL load failed while importing hiredis: The parameter is incorrect.
Command exited with code 1

This is probably related to https://bugs.python.org/issue36085 (new in Python 3.8) (see esp. python/cpython#12302). I'm not familiar with Windows and have no idea how to properly fix this. :(

@ifduyue
Copy link
Collaborator

ifduyue commented Oct 28, 2019

I am not familiar with Windows, too. I tried but without success.
However, when I apply these changes in this PR to another project, it works fine.
Don't know why..

@hugovk
Copy link

hugovk commented Nov 9, 2019

🚀 Python 3.8 is now available on AppVeyor:

@ifduyue
Copy link
Collaborator

ifduyue commented Nov 10, 2019

Workaround no longer needed.
ImportError still exists, need to investigate...

@ifduyue
Copy link
Collaborator

ifduyue commented Nov 13, 2019

I've found the cause, it's in test/__init__.py:

# Add path to hiredis.so load path
path = glob.glob("build/lib*-%s/hiredis" % majorminor)[0]
sys.path.insert(0, path)

This somehow makes Python 3.8 on AppVeyor windows raise ImportError.

@ifduyue ifduyue merged commit cc38c8f into redis:master Nov 13, 2019
@michael-k michael-k deleted the py38 branch November 14, 2019 07:50
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.

3 participants