search

doctrine / KeyValueStore

Abstraction for Key-Value to Plain Old PHP Object mapping
http://www.doctrine-project.org
MIT License
200 stars 59 forks source link

Fix DBALStorage to not ignore exceptions #72

Open enumag opened 8 years ago

enumag commented 8 years ago

Is there some reason to catch all exceptions? If there is a problem with the storage I want to know about it, not silently ignore it.

Ocramius commented 8 years ago

Requires testing. /cc @EmanueleMinotto

EmanueleMinotto commented 8 years ago

@enumag you're right, exceptions should be thrown there as well, anyway without tests a merge won't be safe (I'll cover them asap if you can't).

enumag commented 8 years ago

@EmanueleMinotto I don't have time for that now... Also I've noticed that the DBALStorage completely ignores the $storageName argument. Maybe it should be used as a prefix for the key?

Anyway I've duplicated the class to my project for the time being (without the try-catch blocks).

EmanueleMinotto commented 8 years ago

@enumag yes, I'm reviewing everything in these days

enumag commented 8 years ago

@EmanueleMinotto I'm also wondering if there should be some sort of support for transactions (when inserting/updateing multiple values). Not sure if that's relevant to other storages than sql database.

EmanueleMinotto commented 8 years ago

@enumag it would be great if you could open an issue for every thought :) because $storageName seems a bug, I'm still not sure about transactions and no one of these is related to DBALStorage exceptions

enumag commented 8 years ago

Ok, I'll open separate issues. :-)

enumag commented 8 years ago

All done. You can delete the unrelated commens from this issue if you want.

enumag commented 8 years ago

@EmanueleMinotto Any progress?

EmanueleMinotto commented 8 years ago

@enumag not yet, I'm busy with some things + work, I'll take a look asap