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 PY2 error #180

Closed
wants to merge 7 commits into from
Closed

Fix PY2 error #180

wants to merge 7 commits into from

Conversation

karolyi
Copy link
Contributor

@karolyi karolyi commented Mar 26, 2018

Last year I committed a change, which went red because of PY2 incompatibility. I hereby fix it, please merge ASAP.

@coveralls
Copy link

coveralls commented Mar 26, 2018

Coverage Status

Coverage remained the same at 99.563% when pulling f1861fc on karolyi:master into 3c322a9 on notanumber:master.

@coveralls
Copy link

coveralls commented Mar 26, 2018

Coverage Status

Coverage decreased (-0.4%) to 99.127% when pulling 07c8053 on karolyi:master into 3c322a9 on notanumber:master.

@karolyi
Copy link
Contributor Author

karolyi commented Apr 7, 2018

@jorgecarleitao @notanumber are you still maintaining this project? I have some improvements, if you add me as a committer to this project, I'll add them.

karolyi referenced this pull request Apr 7, 2018
As per https://stackoverflow.com/a/3112717/1067833,
os.path.exists sometimes delivers a wrong value. These are corner
cases, but mine is exactly one:

I run xapian-haystack in a multiprocess environment (indexing and
django server), on an LXC container, which has its mount probably
mounted from an NFS server (I have no control/information over it).

This corner case results the os.path.exists give `True` for one
process and `False` for another, resulting in the exception I
handle with this patch.
@notanumber
Copy link
Owner

Hey @karolyi Thanks for the pull requests. I haven't been the maintainer for this project for several years but if we don't hear from @jorgecarleitao within the next few days I'll go ahead and add you as a contributor.

@karolyi
Copy link
Contributor Author

karolyi commented Apr 7, 2018

@notanumber thx, I'm working with xapian in one of my projects and I had to take a deep dive down the code. I have a lot updates, which will help it out in the long run.

@jorgecarleitao
Copy link
Collaborator

Hi @karolyi. Sorry for the delay, hadn't had much time for looking at it. Just reviewed and pushed your two changes to master. Thanks a lot for them!!!

I haven't bumped the version because the build is failing for Django 1.8. This is a blocker for releasing a new version.

@karolyi
Copy link
Contributor Author

karolyi commented Apr 8, 2018

I intend to fix that, and I have a couple of additions (stopwords, parameterizable weighing algorithms) too. If I you can add me as a contributor, we can pick up some speed too.

@jorgecarleitao
Copy link
Collaborator

jorgecarleitao commented Apr 9, 2018

I am very happy to know that you have a lot of fixes. Could you PR them? I will gladly review them.

@notanumber, it is ok, I am here. Last 2 weeks have been a bit crazy, but things are coming back to normal. I will review the code from @karolyi before it is merged.

karolyi and others added 5 commits October 9, 2018 22:02
Django < 1.11 is no longer supported by Django or Haystack

Test against latest versions of Django 1.11, 2.0, 2.1.
Test against python 2.7 (django 1.11 only), 3.5, 3.6, 3.7 (on xenial).

Update xapian old-stable to 1.2.25, only for py27.
Update xapian old-dev to 1.3.6, only for py27, py34.
Update xapian stable to 1.4.9.

xapian 1.3.6 included just to keep coverage numbers up.
xapian 1.3.7 not included because it didn't build with install-xapian.sh.
This commit could use scrutiny.
check if user supplied version on command line and exit with message if they didn't
@claudep
Copy link
Collaborator

claudep commented Aug 9, 2021

Most of the proposed changes have been merged with other pull requests. If you still find some things remaining to improve, please rebase or propose a new pull request.

@claudep claudep closed this Aug 9, 2021
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.

None yet

7 participants