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

Relax safeguards on delete methods #2039

Merged
merged 1 commit into from Mar 8, 2017

Conversation

Projects
None yet
3 participants
@moradology
Contributor

moradology commented Mar 2, 2017

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.

@moradology moradology changed the title from Relax safeguards on delete methods to Relax safeguards on s3 delete methods Mar 2, 2017

} 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")

This comment has been minimized.

@pomadchin

pomadchin Mar 2, 2017

Member

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.

This comment has been minimized.

@pomadchin

pomadchin Mar 2, 2017

Member

Btw, can this change be done as? A good thing would be add to all backends.

@pomadchin

This comment has been minimized.

Member

pomadchin commented Mar 2, 2017

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

This comment has been minimized.

@pomadchin

pomadchin Mar 2, 2017

Member

can be omited

@moradology moradology changed the title from Relax safeguards on s3 delete methods to Relax safeguards on s3 delete methods (add clobbering writes?): WIP Mar 2, 2017

@pomadchin pomadchin added this to the 1.0.1 milestone Mar 2, 2017

@moradology moradology force-pushed the moradology:feature/quiet-deleter branch from 02a84a5 to 528c385 Mar 7, 2017

sourceLayerPath.delete
}
class FileLayerDeleter(val attributeStore: FileAttributeStore) extends LazyLogging with LayerDeleter[LayerId] {

This comment has been minimized.

@moradology

moradology Mar 7, 2017

Contributor

This is worth taking a close look at - I don't think newing a trait inside an apply is a good idea for readability.

This comment has been minimized.

@pomadchin

pomadchin Mar 8, 2017

Member

what do you mean?

@moradology moradology changed the title from Relax safeguards on s3 delete methods (add clobbering writes?): WIP to Relax safeguards on delete methods Mar 7, 2017

@moradology moradology force-pushed the moradology:feature/quiet-deleter branch from aa1acc8 to 582a07e Mar 7, 2017

import com.datastax.driver.core.querybuilder.QueryBuilder
import com.datastax.driver.core.querybuilder.QueryBuilder.{eq => eqs}
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] {

This comment has been minimized.

@pomadchin

pomadchin Mar 8, 2017

Member

Here can be used a generic AttributeStore, only FileLayerDeleter has a restricted attributeStore type here (but it's a local for-tests backend).

@moradology moradology force-pushed the moradology:feature/quiet-deleter branch from 582a07e to 4980646 Mar 8, 2017

Relax safeguards on delete methods
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.

@lossyrob lossyrob merged commit 147c6a2 into locationtech:master Mar 8, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@lossyrob lossyrob removed the in progress label Mar 8, 2017

@lossyrob lossyrob modified the milestones: 1.1, 1.0.1 Mar 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment