You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by GitBox <gi...@apache.org> on 2021/06/28 08:02:35 UTC
[GitHub] [brooklyn-server] algairim opened a new pull request #1192: Couple of unit-tests for the logbook resource and tidy up
algairim opened a new pull request #1192:
URL: https://github.com/apache/brooklyn-server/pull/1192
Signed-off-by: Mykola Mandra <my...@cloudsoftcorp.com>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] iuliana merged pull request #1192: Couple of unit-tests for the logbook resource and tidy up
Posted by GitBox <gi...@apache.org>.
iuliana merged pull request #1192:
URL: https://github.com/apache/brooklyn-server/pull/1192
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] algairim commented on a change in pull request #1192: Couple of unit-tests for the logbook resource and tidy up
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #1192:
URL: https://github.com/apache/brooklyn-server/pull/1192#discussion_r659998696
##########
File path: rest/rest-resources/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java
##########
@@ -143,14 +143,18 @@ private TestShutdownHandler createShutdownHandler() {
return new TestShutdownHandler();
}
+ protected BrooklynProperties getBrooklynProperties() {
+ return null;
+ }
Review comment:
This allows to introduce Brooklyn properties at instantiation stage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] algairim commented on a change in pull request #1192: Couple of unit-tests for the logbook resource and tidy up
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #1192:
URL: https://github.com/apache/brooklyn-server/pull/1192#discussion_r659998006
##########
File path: core/src/main/java/org/apache/brooklyn/util/core/logbook/DelegatingLogStore.java
##########
@@ -66,7 +66,8 @@ private LogStore loadDelegate() {
Class<? extends LogStore> clazz = (Class<? extends LogStore>) clu.loadClass(className);
delegate = createLogStoreProviderInstance(mgmt, clazz);
} catch (Exception e) {
- log.warn("Brooklyn Logbook: unable to instantiate Log Store " + className, ". Forcing to use FileLogStore", e);
+ log.warn("Brooklyn Logbook: unable to instantiate Log Store '" + className + "'. Fall back on FileLogStore", e.getMessage());
+ log.debug("", e);
Review comment:
Print full stack trace at debug log level only.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] jcabrerizo commented on pull request #1192: Couple of unit-tests for the logbook resource and tidy up
Posted by GitBox <gi...@apache.org>.
jcabrerizo commented on pull request #1192:
URL: https://github.com/apache/brooklyn-server/pull/1192#issuecomment-870283128
This looks good to me @algairim, happy to merge once the tests pass
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] jcabrerizo commented on a change in pull request #1192: Couple of unit-tests for the logbook resource and tidy up
Posted by GitBox <gi...@apache.org>.
jcabrerizo commented on a change in pull request #1192:
URL: https://github.com/apache/brooklyn-server/pull/1192#discussion_r660320362
##########
File path: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/LogbookResource.java
##########
@@ -38,10 +38,13 @@
@Override
public Response logbookQuery(HttpServletRequest request, LogBookQueryParams params) {
- // TODO discuss about if is ok or not allow only to the root user to see the logs.
Review comment:
Good to me @algairim. Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] algairim commented on a change in pull request #1192: Couple of unit-tests for the logbook resource and tidy up
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #1192:
URL: https://github.com/apache/brooklyn-server/pull/1192#discussion_r659575396
##########
File path: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/LogbookResource.java
##########
@@ -38,10 +38,13 @@
@Override
public Response logbookQuery(HttpServletRequest request, LogBookQueryParams params) {
- // TODO discuss about if is ok or not allow only to the root user to see the logs.
Review comment:
Other than root user is out of scope for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] algairim commented on a change in pull request #1192: Couple of unit-tests for the logbook resource and tidy up
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #1192:
URL: https://github.com/apache/brooklyn-server/pull/1192#discussion_r659628402
##########
File path: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/LogbookResource.java
##########
@@ -38,10 +38,13 @@
@Override
public Response logbookQuery(HttpServletRequest request, LogBookQueryParams params) {
- // TODO discuss about if is ok or not allow only to the root user to see the logs.
Review comment:
cc @jcabrerizo
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] jcabrerizo commented on pull request #1192: Couple of unit-tests for the logbook resource and tidy up
Posted by GitBox <gi...@apache.org>.
jcabrerizo commented on pull request #1192:
URL: https://github.com/apache/brooklyn-server/pull/1192#issuecomment-870283128
This looks good to me @algairim, happy to merge once the tests pass
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] algairim commented on a change in pull request #1192: Couple of unit-tests for the logbook resource and tidy up
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #1192:
URL: https://github.com/apache/brooklyn-server/pull/1192#discussion_r660401657
##########
File path: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/LogbookResource.java
##########
@@ -38,10 +38,13 @@
@Override
public Response logbookQuery(HttpServletRequest request, LogBookQueryParams params) {
- // TODO discuss about if is ok or not allow only to the root user to see the logs.
Review comment:
@jcabrerizo , I tagged you to keep these changes in your records. Please do not waste your personal time on this review :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] jcabrerizo commented on a change in pull request #1192: Couple of unit-tests for the logbook resource and tidy up
Posted by GitBox <gi...@apache.org>.
jcabrerizo commented on a change in pull request #1192:
URL: https://github.com/apache/brooklyn-server/pull/1192#discussion_r660320362
##########
File path: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/LogbookResource.java
##########
@@ -38,10 +38,13 @@
@Override
public Response logbookQuery(HttpServletRequest request, LogBookQueryParams params) {
- // TODO discuss about if is ok or not allow only to the root user to see the logs.
Review comment:
Good to me @algairim. Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] algairim commented on a change in pull request #1192: Couple of unit-tests for the logbook resource and tidy up
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #1192:
URL: https://github.com/apache/brooklyn-server/pull/1192#discussion_r659998006
##########
File path: core/src/main/java/org/apache/brooklyn/util/core/logbook/DelegatingLogStore.java
##########
@@ -66,7 +66,8 @@ private LogStore loadDelegate() {
Class<? extends LogStore> clazz = (Class<? extends LogStore>) clu.loadClass(className);
delegate = createLogStoreProviderInstance(mgmt, clazz);
} catch (Exception e) {
- log.warn("Brooklyn Logbook: unable to instantiate Log Store " + className, ". Forcing to use FileLogStore", e);
+ log.warn("Brooklyn Logbook: unable to instantiate Log Store '" + className + "'. Fall back on FileLogStore", e.getMessage());
+ log.debug("", e);
Review comment:
Print full stack trace at debug log level only.
##########
File path: rest/rest-resources/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java
##########
@@ -143,14 +143,18 @@ private TestShutdownHandler createShutdownHandler() {
return new TestShutdownHandler();
}
+ protected BrooklynProperties getBrooklynProperties() {
+ return null;
+ }
Review comment:
This allows to introduce Brooklyn properties at instantiation stage.
##########
File path: rest/rest-resources/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java
##########
@@ -143,14 +143,18 @@ private TestShutdownHandler createShutdownHandler() {
return new TestShutdownHandler();
}
+ protected BrooklynProperties getBrooklynProperties() {
+ return null;
+ }
Review comment:
This allows to introduce Brooklyn properties at instantiation stage. Override in tests and return properties of interest were needed.
##########
File path: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/LogbookResource.java
##########
@@ -38,10 +38,13 @@
@Override
public Response logbookQuery(HttpServletRequest request, LogBookQueryParams params) {
- // TODO discuss about if is ok or not allow only to the root user to see the logs.
Review comment:
@jcabrerizo , I tagged you to keep these changes in your records. Please do not waste your personal time on this review :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] algairim commented on a change in pull request #1192: Couple of unit-tests for the logbook resource and tidy up
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #1192:
URL: https://github.com/apache/brooklyn-server/pull/1192#discussion_r659998696
##########
File path: rest/rest-resources/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java
##########
@@ -143,14 +143,18 @@ private TestShutdownHandler createShutdownHandler() {
return new TestShutdownHandler();
}
+ protected BrooklynProperties getBrooklynProperties() {
+ return null;
+ }
Review comment:
This allows to introduce Brooklyn properties at instantiation stage. Override in tests and return properties of interest were needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org