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

Postgres cancels import after duplicate feed #54

Closed
tewe opened this issue Mar 27, 2014 · 8 comments
Closed

Postgres cancels import after duplicate feed #54

tewe opened this issue Mar 27, 2014 · 8 comments
Labels

Comments

@tewe
Copy link

tewe commented Mar 27, 2014

  1. add feed X
  2. import OPML also containing feed X, but not in the last position

This works with SQLite, because it isn't a "real" database. With PostgresSQL, the feed after X will raise InternalError on previous_feed = Feed.get(Feed.self_link == self_link).

The reason is that add_feeds_from_file uses a single transaction, which gets aborted when creating X violates the unique constraint. The server accepts no further queries, like the SELECT used by Feed.get.

As discussed in #52, asking an RDBMS for forgiveness won't work.

@tewe
Copy link
Author

tewe commented Mar 28, 2014

I tried removing the with_transaction (not sure why it's there) and it kept happening. As far as I can see peewee doesn't use transactions. I don't know what postgres considers an implicit transaction.

@passiomatic
Copy link
Owner

I cannot reproduce the bug with MySQL but I understand it has its limitation when it comes to "correctness". Now that I review the code I think that "with transaction()" bit can be left off.

@tewe
Copy link
Author

tewe commented Mar 28, 2014

Turns out psycopg2 defaults to wrapping things in transactions. As it's encapsulated by peewee I see no way to disable that.

BTW peewee itself looks like a dangerous hack

@passiomatic
Copy link
Owner

I'm wondering if that Peewee's PostgresqlDatabase.commit_select could help:

Whether to issue a commit after executing a select query. With some engines can prevent implicit transactions from piling up. — http://peewee.readthedocs.org/en/latest/peewee/api.html#database-and-its-subclasses

@tewe
Copy link
Author

tewe commented Mar 28, 2014

Unlikely, as the commit needs to happen between the CREATE and the SELECT. We could commit explicitly in every finally, but I'm thinking you wouldn't like the look of that ;)

tewe added a commit to tewe/coldsweat that referenced this issue Mar 29, 2014
This fixes passiomatic#54 but pretty much prevents us from using explicit transactions.
@tewe
Copy link
Author

tewe commented Mar 29, 2014

As I couldn't find a way to disable psycopg2 opening transactions, I made peewee roll them back whenever an error occurs.

The root problem is that peewee's author didn't know about psycopg2 autocommit, wrote his own autocommit, and then patched things over with commit_select rather than going back.

passiomatic added a commit that referenced this issue Mar 29, 2014
@passiomatic
Copy link
Owner

Awesome, thank you.

@tewe
Copy link
Author

tewe commented Mar 29, 2014

Sorry about all the drama for a two line fix. I forgot that coldsweat only needs to serve a handful of users, usually on its own machine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants