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

Add several features #12

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

Add several features #12

wants to merge 33 commits into from

Conversation

ghost
Copy link

@ghost ghost commented May 4, 2017

The added features mainly include:

  • Add possibility to generate config and use it with a different Sphinx instance.
  • Do not start Sphinx instance during the application startup.
  • Add possibility to set up the Sphinx binary path.
  • Add support for unit tests in the application using django-sphinxql using the local instance.

babky added 17 commits April 27, 2017 13:20
Nowadays sphinxql can connect to hosts specified in connection_params in
the configuration. We can connect to remote sphinx servers. With this
the necessity for generating sphinx.conf during startup is void.
Therefore this functionality is refactored out and a command to do this
is added (generate_shpinx_conf). Also the directories required to place
the file are created as well.
The following tasks were done:

1. Allow the connection to external search daemon to be configured.
2. Run the sphinx on background for unit tests.
3. Do not start sphinx by default - connect to the given instance.
4. Refactor SpinxQuerySet.
 + Do not allow querying with sphinx and django ordering, raise
   NotImplementedError.
5. Reflect the changes in tests.
The improvement lies in better output processing - directly to stdout
if required.
The two mentioned settings are no longer available.
SphinxQlTestCase is added with the setUp and tearDown methods so that
we can start/stop/index sphinx instance on demand.
@jorgecarleitao
Copy link
Owner

Hey. Thanks a lot for this, @babky-atteq. I have not checked specifically what you did, but the tests are not passing. Let me know when you are done so I can review it and merge accordingly.

@ghost
Copy link
Author

ghost commented May 5, 2017

Hello, I'll fix them in a couple of days. They were passing on my computer, and I was not clever enough to check them on Travis. But I'll make them working.

babky added 5 commits May 13, 2017 17:16
Describe how unit test settings can be adapted to a local environment.
Describe sphinx_bin_path usage.
The library was not working with Python 3.4, the compatibility was
added. Also the unit tests now work on slower computers - a limit
of 2 seconds was added to wait until the searchd starts. This can
be further improved to detect a real start of searchd.
@coveralls
Copy link

Coverage Status

Coverage decreased (-5.3%) to 91.166% when pulling 2a537e6 on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

@coveralls
Copy link

coveralls commented May 13, 2017

Coverage Status

Coverage decreased (-4.8%) to 91.673% when pulling adaee2f on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

2 similar comments
@coveralls
Copy link

coveralls commented May 13, 2017

Coverage Status

Coverage decreased (-4.8%) to 91.673% when pulling adaee2f on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.8%) to 91.673% when pulling adaee2f on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

@coveralls
Copy link

coveralls commented May 13, 2017

Coverage Status

Coverage decreased (-4.8%) to 91.673% when pulling 9fdb930 on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.8%) to 91.673% when pulling ad982bf on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

3 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-4.8%) to 91.673% when pulling ad982bf on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.8%) to 91.673% when pulling ad982bf on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

@coveralls
Copy link

coveralls commented May 13, 2017

Coverage Status

Coverage decreased (-4.8%) to 91.673% when pulling ad982bf on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

@ghost
Copy link
Author

ghost commented May 13, 2017

Hello @jorgecarleitao, I've made the tests passing on Travis. You can now check the request and merge if you like.

Notice that I slightly changed the behavior in the way how the sorting is defined. When both sorting in Sphinx and in Django is used an exception is raised so that there's no default behavior or ignoring of one way of sorting. There should be no other interface/contract break.

The coverage drop is because of the added commands to generate sphinx configuration.

Aspect26 and others added 2 commits May 17, 2017 10:55
@coveralls
Copy link

Coverage Status

Coverage decreased (-4.8%) to 91.673% when pulling 77bae80 on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

6 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-4.8%) to 91.673% when pulling 77bae80 on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.8%) to 91.673% when pulling 77bae80 on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.8%) to 91.673% when pulling 77bae80 on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.8%) to 91.673% when pulling 77bae80 on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

@coveralls
Copy link

coveralls commented May 17, 2017

Coverage Status

Coverage decreased (-4.8%) to 91.673% when pulling 77bae80 on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.8%) to 91.673% when pulling 77bae80 on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

@coveralls
Copy link

coveralls commented May 24, 2017

Coverage Status

Coverage decreased (-4.9%) to 91.625% when pulling 19d85e8 on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

@ghost
Copy link
Author

ghost commented Jun 7, 2017

We also added a possibility to define the table/index name via

class Index(indexes.Index):
    class Meta:
        db_table= '<index_name>'

@jorgecarleitao
Copy link
Owner

I have not forgotten this. It is very solid work. I would like to review it before pushing to the master, and will probably make the history linear. Regardless, great work @babky-atteq

@coveralls
Copy link

coveralls commented Jun 14, 2017

Coverage Status

Coverage decreased (-4.9%) to 91.625% when pulling 7f3939c on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

@coveralls
Copy link

coveralls commented Jul 4, 2017

Coverage Status

Coverage decreased (-4.9%) to 91.625% when pulling 286d60f on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

@coveralls
Copy link

coveralls commented Aug 26, 2017

Coverage Status

Coverage decreased (-4.9%) to 91.637% when pulling f6ab9ea on AtteqCom:master into 3f42f51 on jorgecarleitao:master.

@geoolekom
Copy link

What about this feature? Can you merge it?

@babky
Copy link

babky commented Nov 11, 2018

^^ @jorgecarleitao: we would also be happy to use a pypi package instead of github repo reference 👍 If required, we could make some improvements.

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

5 participants