add() should not commit by default about pysolr HOT 19 CLOSED

@mlissner I'm marking this a 4.0.0 since it seems like the kind of thing we should feature prominently in the release notes.

from pysolr.

@efagerberg That seems reasonable if you want to send a pull request - change the default and document how to change it back should you depend on it in some way.

from pysolr.

Before I make that change to the PR I want to make sure the maintainer is ok with that. @acdha do you have any thoughts?

from pysolr.

I should have put that in the message — it's 30 days after the message to confirm that an issue is still relevant:

https://github.com/django-haystack/pysolr/blob/master/.github/stale.yml#L4

It seems like between this, #94, etc. it might be worth rolling a 4.0 release with a few of these breaking cleanups.

from pysolr.

Sorry, this is probably a wontfix as it is very backward-incompatible. Most usage I've seen sends a fair number of docs (500-1000 docs) at a time, so the default "autocommit" makes a lot of sense. The commit=... kwarg is there on every data manipulation method, so you still have the option not to do it.

from pysolr.

I understand. I think the least you could do is make the documentation exceptionally clear when PySolr will send a commit automatically.

from pysolr.

Agreed on that. Reopening as a docs ticket.

from pysolr.

I'd like to revisit this conversation and get commit=False put on the roadmap for the next big version of pysolr.

Here's my argument:

1. This isn't documented, even though it should have been four years ago.
2. At some point this change should be made.
3. Coming from other libraries, every one of them has an add method and I've never seen commit=True prior to this.
4. If you ever use add without remembering to do commit=True on a big system, you've made a grievous error (this shouldn't be the default).

I recognize that this is a major disruption for people, but I think we should bite the bullet and do it. It's easy to upgrade your code to add commit=True wherever you have an add method in the past.

from pysolr.

Fantastic, @acdha. That was my hope!

from pysolr.

@acdha Perhaps a good solution is to somehow make the default commit value be configurable on the PySolr object level or module level. That way we can still support older solr versions.

from pysolr.

PR here: #221

from pysolr.

I gave a thumbs up to this approach earlier, but I'm looking at the now, and I really don't like it. I especially don't like how the add function has an argument that can be True, False, or None.

Seems like if we're doing a version bump, we should accept some backwards breakage, and I don't think we need to support the way it used to work, if it means we'll forever have a more complex API, more documentation, more tests, etc. I say break with the past, make a note in the release notes, and move forward with the right way.

from pysolr.

@mlissner so the PR I posted does incure some backwards breakage already as you have to explicitly set an option in the Solr object. I use the presence of commit=None to signal that the user has not supplied that arg, otherwise it would be impossible to tell if the user explicitly doesn't want to commit something.

I don't think of it solely as supporting the past at this point because it still allows some simplicity in writing the code if you want to commit you do not have to write two lines, you can do it in one.

I am fine with not allowing an option in the constructor to go back to similar functionality, but we would probably want that functionality to specify when to commit in the add function as syntactic sugar.

from pysolr.

I am fine with not allowing an option in the constructor to go back to similar functionality, but we would probably want that functionality to specify when to commit in the add function as syntactic sugar.

Yeah, that's what I'd suggest too. Basically, add gets a new option about committing, defaults to not committing, and that's it.

The use case for setting up a SolrInterface, and then wanting every add to do a commit seems small to me. I think other libraries (FWIW) have commit as a separate call altogether.

from pysolr.

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

from pysolr.

If pysolr still has life, this should most definitely not be closed.

It'd be nice if the stale bot could say how long you had until something was automatically closed.

from pysolr.

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

from pysolr.

Hey folks, I have rebased on the master branch so my PR should be up to date, please let me know if there is any other feedback you have to make it an acceptable contribution.

from pysolr.

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

from pysolr.