Repository navigation
Relax safeguards on delete methods - #2039
Conversation
moradology
changed the title
| } catch { | ||
| case e: AttributeNotFoundError => throw new LayerDeleteError(id).initCause(e) | ||
| case e: AttributeNotFoundError => | ||
| logger.info(s"Metadata for $id was not found. Any associated layer data (if any) will require manual deletion") |
There was a problem hiding this comment.
I believe we already had similar way of wrapping errors (SomeException.initcause() ...), probably you remember it; should consider changing the behaviour of all backends? As currently all layer deleters throw LayerNotFoundError in case of missing layer, and native s3 errors in other cases. Don't forget that all layer managements functions change should be consistent across all backends.
There was a problem hiding this comment.
Btw, can this change be done as? A good thing would be add to all backends.
|
Eh i didn't understand immediately that you want to improve logging and md deletion, my bad :D |
| case e: AttributeNotFoundError => | ||
| logger.info(s"Metadata for $id was not found. Any associated layer data (if any) will require manual deletion") | ||
| case e: Exception => | ||
| throw e |
moradology
changed the title
02a84a5 to
528c385
Compare
|
|
||
| sourceLayerPath.delete | ||
| } | ||
| class FileLayerDeleter(val attributeStore: FileAttributeStore) extends LazyLogging with LayerDeleter[LayerId] { |
There was a problem hiding this comment.
This is worth taking a close look at - I don't think newing a trait inside an apply is a good idea for readability.
moradology
changed the title
aa1acc8 to
582a07e
Compare
| import scala.collection.JavaConversions._ | ||
|
|
||
| class CassandraLayerDeleter(val attributeStore: AttributeStore, instance: CassandraInstance) extends LayerDeleter[LayerId] { | ||
| class CassandraLayerDeleter(val attributeStore: CassandraAttributeStore, instance: CassandraInstance) extends LazyLogging with LayerDeleter[LayerId] { |
There was a problem hiding this comment.
Here can be used a generic AttributeStore, only FileLayerDeleter has a restricted attributeStore type here (but it's a local for-tests backend).
582a07e to
4980646
Compare
Prior to this change, delete methods threw in any case where the reference to a layer happened to be faulty. This is based on metadata which, though always present in case of tile data (and necessary for its deletion), might not be present. Lacking this metadata, we should log information about the failure and delete any _attribute data we can.
3 participants
Prior to this change,
deletemethods threw in any case where the reference to a layer happened to be faulty. This is based on metadata which, though always present in case of tile data (and necessary for its deletion), might not be present. Lacking this metadata, we should log information about the failure and delete any_attributedata we can.