You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2021/09/13 10:44:15 UTC
[GitHub] [sling-org-apache-sling-distribution-core] mohiaror opened a new pull request #55: SLING-10602 DistributionQueueItem's path should be correctly logged in case of success and failure
mohiaror opened a new pull request #55:
URL: https://github.com/apache/sling-org-apache-sling-distribution-core/pull/55
Adding the path(s) for which distribution package was created in the logging for distribution queue item. Also changing the debug level log to info level to give admin more info about which resources were distributed and how much time it took for distribution to complete.
--
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@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-distribution-core] cschneider merged pull request #55: SLING-10602 DistributionQueueItem's path should be correctly logged in case of success and failure
Posted by GitBox <gi...@apache.org>.
cschneider merged pull request #55:
URL: https://github.com/apache/sling-org-apache-sling-distribution-core/pull/55
--
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@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-distribution-core] mohiaror commented on pull request #55: SLING-10602 DistributionQueueItem's path should be correctly logged in case of success and failure
Posted by GitBox <gi...@apache.org>.
mohiaror commented on pull request #55:
URL: https://github.com/apache/sling-org-apache-sling-distribution-core/pull/55#issuecomment-918223007
The sprouts is failing to build the project and failing with following exception. It seems the machine where sprouts is running has java 11 and the bundle does not build with that.
```
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Skipping Apache Sling Distribution Core
[INFO] This project has been banned from the build due to previous failures.
[INFO] ------------------------------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 5.609 s
[INFO] Finished at: 2021-09-13T10:49:17Z
[INFO] ------------------------------------------------------------------------
[WARNING] The requested profile "ci" could not be activated because it does not exist.
[INFO] [jenkins-event-spy] Generated /home/jenkins/workspace/he-sling-distribution-core_PR-55@tmp/withMaven442fa326/maven-spy-20210913-104911-2979981949278491312987.log
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-antrun-plugin:1.8:run (set-bundle-required-execution-environment) on project org.apache.sling.distribution.core: An Ant BuildException has occured: Unable to create javax script engine for javascript
[ERROR] around Ant part ...<script language="javascript">var System = java.lang.System;... @ 4:33 in /home/jenkins/workspace/he-sling-distribution-core_PR-55/target/antrun/build-main.xml
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
```
--
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@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-distribution-core] mohiaror commented on pull request #55: SLING-10602 DistributionQueueItem's path should be correctly logged in case of success and failure
Posted by GitBox <gi...@apache.org>.
mohiaror commented on pull request #55:
URL: https://github.com/apache/sling-org-apache-sling-distribution-core/pull/55#issuecomment-919108158
@cschneider Thanks for approving the PR. Can you please also merge it? Can you please also cut an internal release (if not an official one).
--
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@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-distribution-core] mohiaror commented on a change in pull request #55: SLING-10602 DistributionQueueItem's path should be correctly logged in case of success and failure
Posted by GitBox <gi...@apache.org>.
mohiaror commented on a change in pull request #55:
URL: https://github.com/apache/sling-org-apache-sling-distribution-core/pull/55#discussion_r708009566
##########
File path: src/main/java/org/apache/sling/distribution/agent/impl/SimpleDistributionAgentQueueProcessor.java
##########
@@ -92,7 +92,7 @@ public boolean process(@NotNull String queueName, @NotNull DistributionQueueEntr
final long endTime = System.currentTimeMillis();
- distributionLog.debug("[{}] ITEM-PROCESSED item={}, status={}, processingTime={}ms", queueName, queueItem, success, endTime - startTime);
+ distributionLog.info("[{}] ITEM-PROCESSED item={}, status={}, processingTime={}ms", queueName, queueItem, success, endTime - startTime);
Review comment:
@cschneider thanks for the review. My motivation behind the change was to also log the queueItemID in case of successful processing of item. The item ID is currently only logged at debug level or at error/warn level in case of an exception. I have updated the existing log info for successful distribution of item and added the queue item ID to it - https://github.com/apache/sling-org-apache-sling-distribution-core/pull/55/commits/33b54a5a56381fd7a3c60db46d6001cbf5426780
Would this be an acceptable change?
--
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@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-distribution-core] cschneider commented on a change in pull request #55: SLING-10602 DistributionQueueItem's path should be correctly logged in case of success and failure
Posted by GitBox <gi...@apache.org>.
cschneider commented on a change in pull request #55:
URL: https://github.com/apache/sling-org-apache-sling-distribution-core/pull/55#discussion_r707948885
##########
File path: src/main/java/org/apache/sling/distribution/agent/impl/SimpleDistributionAgentQueueProcessor.java
##########
@@ -92,7 +92,7 @@ public boolean process(@NotNull String queueName, @NotNull DistributionQueueEntr
final long endTime = System.currentTimeMillis();
- distributionLog.debug("[{}] ITEM-PROCESSED item={}, status={}, processingTime={}ms", queueName, queueItem, success, endTime - startTime);
+ distributionLog.info("[{}] ITEM-PROCESSED item={}, status={}, processingTime={}ms", queueName, queueItem, success, endTime - startTime);
Review comment:
We should not change the log level as it would lead to a lot of additional log messages.
--
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@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-distribution-core] mohiaror commented on a change in pull request #55: SLING-10602 DistributionQueueItem's path should be correctly logged in case of success and failure
Posted by GitBox <gi...@apache.org>.
mohiaror commented on a change in pull request #55:
URL: https://github.com/apache/sling-org-apache-sling-distribution-core/pull/55#discussion_r708009566
##########
File path: src/main/java/org/apache/sling/distribution/agent/impl/SimpleDistributionAgentQueueProcessor.java
##########
@@ -92,7 +92,7 @@ public boolean process(@NotNull String queueName, @NotNull DistributionQueueEntr
final long endTime = System.currentTimeMillis();
- distributionLog.debug("[{}] ITEM-PROCESSED item={}, status={}, processingTime={}ms", queueName, queueItem, success, endTime - startTime);
+ distributionLog.info("[{}] ITEM-PROCESSED item={}, status={}, processingTime={}ms", queueName, queueItem, success, endTime - startTime);
Review comment:
@cschneider thanks for the review. My motivation behind the change was to also log the queueItemID in case of successful processing of item. The item ID is currently only logged in debug level or at error/warn level in case of an exception. I have updated the existing log info for successful distribution of item and added the queue item ID to it - https://github.com/apache/sling-org-apache-sling-distribution-core/pull/55/commits/33b54a5a56381fd7a3c60db46d6001cbf5426780
Would this be an acceptable change?
--
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@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-distribution-core] actinium15 commented on a change in pull request #55: SLING-10602 DistributionQueueItem's path should be correctly logged in case of success and failure
Posted by GitBox <gi...@apache.org>.
actinium15 commented on a change in pull request #55:
URL: https://github.com/apache/sling-org-apache-sling-distribution-core/pull/55#discussion_r707371685
##########
File path: src/main/java/org/apache/sling/distribution/queue/DistributionQueueItem.java
##########
@@ -64,7 +67,25 @@ public long getSize() {
public String toString() {
return "DistributionQueueItem{" +
"id='" + packageId + '\'' +
- ", info=" + super.toString() +
+ ", info={" + queueInfo(base) + '}' +
'}';
}
+
+ /*
+ * convert the map of object values into string form
+ */
+ private String queueInfo(Map<String, Object> base) {
+ String queueItem = "";
+ for(String key : base.keySet()) {
+ Object value = base.get(key);
+ String valueString = "";
+ if (value instanceof String[]) {
+ valueString = key + "=" + Arrays.toString((String[])value);
Review comment:
```suggestion
if (value.getClass().isArray()) {
valueString = key + "=" + Arrays.toString((Object[])value);
```
##########
File path: src/main/java/org/apache/sling/distribution/queue/DistributionQueueItem.java
##########
@@ -44,7 +47,7 @@ public DistributionQueueItem(String packageId, long size, Map<String, Object> ba
super(base);
this.packageId = packageId;
this.size = size;
-
+ this.base = base;
Review comment:
I think we shouldn't keep this field because
1. it is unnecessary
2. it is possible to invoke `.put` directly on `this` because it extends `ValueMapDecorator`, which can make `base` out of sync with what's there in `DistributionQueueItem` object
##########
File path: src/main/java/org/apache/sling/distribution/queue/DistributionQueueItem.java
##########
@@ -64,7 +67,25 @@ public long getSize() {
public String toString() {
return "DistributionQueueItem{" +
"id='" + packageId + '\'' +
- ", info=" + super.toString() +
+ ", info={" + queueInfo(base) + '}' +
'}';
}
+
+ /*
+ * convert the map of object values into string form
+ */
+ private String queueInfo(Map<String, Object> base) {
+ String queueItem = "";
Review comment:
```suggestion
String queueItemStr = "";
```
--
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@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-distribution-core] mohiaror commented on a change in pull request #55: SLING-10602 DistributionQueueItem's path should be correctly logged in case of success and failure
Posted by GitBox <gi...@apache.org>.
mohiaror commented on a change in pull request #55:
URL: https://github.com/apache/sling-org-apache-sling-distribution-core/pull/55#discussion_r708010395
##########
File path: src/main/java/org/apache/sling/distribution/agent/impl/SimpleDistributionAgentQueueProcessor.java
##########
@@ -92,7 +92,7 @@ public boolean process(@NotNull String queueName, @NotNull DistributionQueueEntr
final long endTime = System.currentTimeMillis();
- distributionLog.debug("[{}] ITEM-PROCESSED item={}, status={}, processingTime={}ms", queueName, queueItem, success, endTime - startTime);
+ distributionLog.info("[{}] ITEM-PROCESSED item={}, status={}, processingTime={}ms", queueName, queueItem, success, endTime - startTime);
Review comment:
This is what the distribution lon entry would look like now in case of successful distribution -
`2021/09/14 13:21:02:084 - INFO - [queue-bpdistributionagent0] PACKAGE-DELIVERED DSTRQ2: ADD item=dstrpck-1631605857371-3241eb55-0949-4ece-953d-4ebbf2ebf81d, paths=[/content/dam/10683CD3.JPG], importTime=4627ms, execTime=4739ms, size=10611B`
##########
File path: src/main/java/org/apache/sling/distribution/agent/impl/SimpleDistributionAgentQueueProcessor.java
##########
@@ -92,7 +92,7 @@ public boolean process(@NotNull String queueName, @NotNull DistributionQueueEntr
final long endTime = System.currentTimeMillis();
- distributionLog.debug("[{}] ITEM-PROCESSED item={}, status={}, processingTime={}ms", queueName, queueItem, success, endTime - startTime);
+ distributionLog.info("[{}] ITEM-PROCESSED item={}, status={}, processingTime={}ms", queueName, queueItem, success, endTime - startTime);
Review comment:
This is what the distribution log entry would look like now in case of successful distribution -
`2021/09/14 13:21:02:084 - INFO - [queue-bpdistributionagent0] PACKAGE-DELIVERED DSTRQ2: ADD item=dstrpck-1631605857371-3241eb55-0949-4ece-953d-4ebbf2ebf81d, paths=[/content/dam/10683CD3.JPG], importTime=4627ms, execTime=4739ms, size=10611B`
--
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@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-distribution-core] mohiaror commented on a change in pull request #55: SLING-10602 DistributionQueueItem's path should be correctly logged in case of success and failure
Posted by GitBox <gi...@apache.org>.
mohiaror commented on a change in pull request #55:
URL: https://github.com/apache/sling-org-apache-sling-distribution-core/pull/55#discussion_r708009566
##########
File path: src/main/java/org/apache/sling/distribution/agent/impl/SimpleDistributionAgentQueueProcessor.java
##########
@@ -92,7 +92,7 @@ public boolean process(@NotNull String queueName, @NotNull DistributionQueueEntr
final long endTime = System.currentTimeMillis();
- distributionLog.debug("[{}] ITEM-PROCESSED item={}, status={}, processingTime={}ms", queueName, queueItem, success, endTime - startTime);
+ distributionLog.info("[{}] ITEM-PROCESSED item={}, status={}, processingTime={}ms", queueName, queueItem, success, endTime - startTime);
Review comment:
@cschneider thanks for the review. My motivation behind the change was to also log the queueItemID in case of successful processing of item. The item ID is currently only logged at debug level or at error/warn level in case of an exception. I have updated the existing info log for successful distribution of item and added the queue item ID to it - https://github.com/apache/sling-org-apache-sling-distribution-core/pull/55/commits/33b54a5a56381fd7a3c60db46d6001cbf5426780
And I have moved this log back to debug level. Would this be an acceptable change?
--
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@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org