search

passiomatic / coldsweat

Web RSS aggregator and reader compatible with the Fever API
MIT License
146 stars 21 forks source link

Postgres cancels import after duplicate feed #54

Closed tewe closed 10 years ago

tewe commented 10 years ago
  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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

Awesome, thank you.

tewe commented 10 years ago

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.