-
Notifications
You must be signed in to change notification settings - Fork 5
Fixes #11412: Can not delete a node property set by datasources #29
Fixes #11412: Can not delete a node property set by datasources #29
Conversation
|
||
case Post( "reload" :: Nil, req) => { | ||
implicit val prettify = extractor.extractPrettify(req.params) | ||
implicit val action = "reloadAllDatasourcesAllNodes" | ||
val actor = RestUtils.getActor(req) | ||
|
||
apiService.reloadDataAllNodes(actor) |
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.
WHy do you remove it ?
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.
hum. No idea, it was not supposed to be done.
@@ -9,5 +9,6 @@ target* | |||
.externalToolBuilders | |||
datasources/ | |||
files.txz | |||
scripts.txz |
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.
Why do you include scripts.txz ?
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.
because it was not included in the gitignore but is a side product of compilation. It must not be commited.
Two litle remarks that are not related with the fix Nice fix though, Nice it can clean 'deleted' datasource! |
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.
some simple messages to change. The rest looks good to me, but Vincent should have a look
|
||
apiService.clearDataAllNodesFor(actor, DataSourceId(datasourceId)) match { | ||
case Full(_) => toJsonResponse(None, JString(s"Data for all nodes, for data source '${datasourceId}', cleared")) | ||
case eb:EmptyBox => toJsonError(None, JString((eb ?~! "Could not clear data source property").messageChain)) |
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.
Could you add the datasourceId in the message ?
@@ -130,7 +152,7 @@ class DataSourceApi9 ( | |||
} | |||
|
|||
case Delete(sourceId :: Nil, req) => { | |||
response(apiService.deleteSource(DataSourceId(sourceId)), req, "Could not delete data sources", None)("getDataSource") | |||
response(apiService.deleteSource(DataSourceId(sourceId)), req, "Could not delete data sources", None)("deteteDataSource") |
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.
Could you add also here the data source id ?
PR rebased |
5721872
to
bbf3034
Compare
OK, merging this PR |
https://www.rudder-project.org/redmine/issues/11412