gtfspy icon indicating copy to clipboard operation
gtfspy copied to clipboard

Fix unique constraint error

Open evelyn9191 opened this issue 5 years ago • 6 comments

This commit fixes situation when the database is already filled with data and the user tries to add them again. In such a case, the script failed with IntegrityError: UNIQUE constraint failed when inserting a value

evelyn9191 avatar Jan 25 '20 15:01 evelyn9191

I am curious of in which situations one would like to add the same content into the database? If I recall correctly the original idea has been not to accept this behavior.

Thank you for your contributions!

jweckstr avatar Jan 31 '20 11:01 jweckstr

I also seem to remember being designed as a one-shot thing. But your changes are exactly what I would do to allow incremental updating. I guess one use case would be "create the DB, get newer version of gtfs a week later, incrementally update" ?

I'd be careful that no other bugs appear on multiple import. For example, post-processing creating duplicate structures or multiple IDs for the same object.

I'm cautiously optimistic about this, but if you're using this successfully, then that makes me positive.

rkdarst avatar Jan 31 '20 11:01 rkdarst

Yes, @rkdarst, that is exactly my use case. The GTFS data are updated once per week. My app using this library would be working offline, but when a user is online and the new data are available, they will be downloaded. I prefer upserting the data rather than deleting the current DB, because I think that in case of a failed update for any reason, the user should have access to the old (even if half-way updated) data rather than no data, as it will make it impossible for him to search a connection.

I haven't tested this yet on the post-processing as I cannot get the tests working on Windows, so, for the time being, I would leave this PR to your suggestion.

evelyn9191 avatar Feb 01 '20 18:02 evelyn9191

Rebased onto master branch.

evelyn9191 avatar Feb 01 '20 18:02 evelyn9191

So, I think this is a good idea, assuming it works and makes no other problems. I can't think of any...

Does it work for you with various tests, no corruption from re-importing something that already is in there? If so, let's do it.

rkdarst avatar Mar 02 '20 23:03 rkdarst

I think we should do this, it will help someone and won't hurt those doing it the old way. If problems appear later, we can fix or revert.

I just added a changelog entry (and unfortunately had to merge master in - I didn't want to rebase your branch, but feel free to if you want). Could you add a test of some sort - I don't know just what, but you can probably figure out sooner than me. Even a simple thing that tries to load the same file twice would work. Can you think of easy, more advanced tests to add?

rkdarst avatar Mar 11 '20 13:03 rkdarst