Skip to content
This repository has been archived by the owner on May 27, 2020. It is now read-only.

fix reading properties without changing old configuration #119

Merged
merged 7 commits into from
Apr 27, 2016
Merged

fix reading properties without changing old configuration #119

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
99e152f
fix reading properties
wuciawe Apr 21, 2016
62ec418
fix reading properties
wuciawe Apr 25, 2016
cb32a90
filterKeys doesn't return a Serializable Map
wuciawe Apr 25, 2016
9525121
Fixed broken filter
pfcoperez Apr 27, 2016
f23ecd6
Improved style
pfcoperez Apr 27, 2016
c9c7bad
Fixed spark incompatibility
pfcoperez Apr 27, 2016
4480f0c
Merge pull request #2 from pfcoperez/pr119_suggestions
wuciawe Apr 27, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion ...ongodb/src/main/scala/com/stratio/datasource/mongodb/partitioner/MongodbPartitioner.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ class MongodbPartitioner(config: Config) extends Partitioner[MongodbPartition] {
@transient private val ssloptions: Option[MongodbSSLOptions] =
config.get[MongodbSSLOptions](MongodbConfig.SSLOptions)

private val clientOptions = config.properties //config.properties.filterKeys(_.contains(MongodbConfig.ListMongoClientOptions)) // TODO review this Map. Can't filter keys
private val clientOptions = {
val lowerCaseOptions = MongodbConfig.ListMongoClientOptions.map(_.toLowerCase)
config.properties.filter(kv => kv._1.contains(lowerCaseOptions))
Copy link
Contributor

@pfcoperez pfcoperez Apr 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic here is wrong: You are filtering those config properties which does not contain the whole option list.

I'd try something like:

lowerCaseOptions.flatMap(key => config.properties.get(key).map(key -> _)) toMap

Or, cleaner:

private val clientOptions = {
    val lowerCaseOptions = MongodbConfig.ListMongoClientOptions.map(_.toLowerCase).toSet
    config.properties.filterKeys(lowerCaseOptions contains _)
  }

Copy link
Contributor

@pfcoperez pfcoperez Apr 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at your commits I see you had already noticed that filterKeys isn't compatible with a map to be distributed to Spark workers as its return instance is not serializable. In any case, you should apply the logic I proposed:

 private val clientOptions = {
   val lowerCaseOptions = MongodbConfig.ListMongoClientOptions.map(_.toLowerCase).toSet
   config.properties.filter { case (k, _) => lowerCaseOptions contains k }
 }

}

private val databaseName: String = config(MongodbConfig.Database)

Expand Down
6 changes: 3 additions & 3 deletions spark-mongodb/src/main/scala/com/stratio/datasource/util/Config.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ abstract class ConfigBuilder[Builder<:ConfigBuilder[Builder] ](
*/
def build(): Config = new Config {

val properties = builder.properties
val properties = builder.properties.map(kv => kv.copy(_1 = kv._1.toLowerCase))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach is more readable:

val properties = builder.properties.map { case (k, v) => k.toLowerCase -> v }


require(
requiredProperties.forall(properties.isDefinedAt),
Expand Down Expand Up @@ -109,7 +109,7 @@ trait Config extends Serializable {
* @return the value associated with `key` if it exists,
* otherwise the result of the `default` computation.
*/
def getOrElse[T](key: Property, default: => T): T = properties.get(key) match {
def getOrElse[T](key: Property, default: => T): T = properties.get(key.toLowerCase) match {
case Some(v) => v.asInstanceOf[T]
case None => default
}
Expand All @@ -121,7 +121,7 @@ trait Config extends Serializable {
* @return An optional value of expected type
*/
def get[T: ClassTag](property: Property): Option[T] =
properties.get(property).map(_.asInstanceOf[T])
properties.get(property.toLowerCase).map(_.asInstanceOf[T])

/**
* Gets specified property from current configuration object.
Expand Down