-
Notifications
You must be signed in to change notification settings - Fork 5
Fixes #10351: Make datasources behaviour on 404 request configurable #12
Conversation
Can you rebase in one commit ? |
@@ -230,6 +230,61 @@ <h4 class="no-margin"> | |||
</ul> | |||
</div> | |||
</div> | |||
<div class="form-group"> | |||
<label class="bloc"><a ng-click="toggleWell($event)">What to do when a queries for a node returns Error 404 (missing)?<span class="fa fa-chevron-right"></span></a></label> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What to do when a query for a Node returns a 404 error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you prefer
def extractMissingNodeBehavior(json: JValue) : Box[M[MissingNodeBehavior]] = { | ||
val onMissing: Box[MissingNodeBehavior] = (json \ "onMissing" match { | ||
case JNothing => Full(MissingNodeBehavior.Delete) | ||
case JObject(fields) => fields.find( _.name == "name" ) match { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would:
case obj : JObject => obj \ "name" match { case JString(Delete.name) => Full(Delete) ... case JString(Default.name) => obj \ "value" match { case JNothing => Failure case value => value } }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
//we only get the first element from the path. | ||
//if list is empty, return "" (remove property). | ||
DataSource.nodeProperty(datasourceName.value, json.headOption.getOrElse("")) | ||
json.map( list => DataSource.nodeProperty(datasourceName.value, list.headOption.getOrElse("")) ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to read it thrice but it's ok :) ( In first attempt, i thought the None (NoChange) would be transformed into "" and would Delete the property but understood !!
MyDatasource.http.queryAll(datasource, c) match { | ||
case Full(res) => //ok | ||
case x => logger.error(s"oh no! Got a $x") | ||
// "Update on datasource" should { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want it to be commented ??? I think we should remove if so
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope :) it was because it took to long when testing, and eclipse does not help a lot to start only one test
PR rebased |
873efba
to
6b18d7f
Compare
PR rebased |
6b18d7f
to
361d863
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
OK, merging this PR |
https://www.rudder-project.org/redmine/issues/10351