Skip to content
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

DataSetInfo parser #175

Merged
merged 17 commits into from
Aug 23, 2016
Merged

Conversation

MoniMoledo
Copy link
Contributor

Implement DataSetInfo parser to and from a JSON Record

@JavierJia

@codecov-io
Copy link

codecov-io commented Aug 18, 2016

Current coverage is 58.70% (diff: 91.61%)

Merging #175 into master will increase coverage by 2.00%

@@             master       #175   diff @@
==========================================
  Files            59         60     +1   
  Lines          1755       1862   +107   
  Methods        1673       1780   +107   
  Messages          0          0          
  Branches         82         82          
==========================================
+ Hits            995       1093    +98   
- Misses          760        769     +9   
  Partials          0          0          

Powered by Codecov. Last update 7a65c09...a1f1694


implicit val intervalFormat: Format[Interval] = new Format[Interval] {
override def reads(json: JsValue) = {
val start = (json \ "start").as[DateTime]
Copy link
Contributor

Choose a reason for hiding this comment

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

to avoid the timezone related parsing problems, usually we just store the getMillis as Long other than DateTime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now, I let this as Datetime like in Query as we discussed.

@JavierJia
Copy link
Contributor

Looks good. Just had some minor comments.

}
}

implicit def tuple2Writes: Writes[Tuple2[String, String]] = Writes[(String, String)](t => Json.obj("level" -> t._1, "field" -> t._2))
Copy link
Contributor

Choose a reason for hiding this comment

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

could you add the comment to let reader know who is using it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

case basicField: Field => JsObject(List(
"name" -> JsString(name),
"isOptional" -> JsBoolean(isOptional),
"datatype" -> JsString(dataType)))
Copy link
Contributor

@JavierJia JavierJia Aug 22, 2016

Choose a reason for hiding this comment

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

add the default throws in case we hit the unhandled fields?
caes any: Field => throw xxxx or simply case _ => ???

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

implicit def tuple2Writes: Writes[Tuple2[String, String]] = Writes[(String, String)](t => Json.obj("level" -> t._1, "field" -> t._2))

def parseLevels(levelSeq: Seq[Map[String, String]]): Seq[(String, String)] = {
levelSeq.map {
levelMap => (levelMap.get("level").getOrElse(""), levelMap.get("field").getOrElse(""))
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

just let it throw :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@JavierJia
Copy link
Contributor

👍

@MoniMoledo MoniMoledo merged commit 58c310a into ISG-ICS:master Aug 23, 2016
@MoniMoledo MoniMoledo deleted the datasetinfo_jsrecord branch August 23, 2016 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants