jnioche commented on code in PR #2050:
URL: https://github.com/apache/stormcrawler/pull/2050#discussion_r3819704186
##########
core/src/main/java/org/apache/stormcrawler/persistence/AbstractStatusUpdaterBolt.java:
##########
@@ -214,15 +225,24 @@ public void execute(Tuple tuple) {
if (!status.equals(Status.FETCH_ERROR)) {
metadata.remove(Constants.fetchErrorCountParamName);
}
+
// https://github.com/apache/stormcrawler/issues/415
// remove error related key values in case of success
if (status.equals(Status.FETCHED) ||
status.equals(Status.REDIRECTION)) {
metadata.remove(Constants.STATUS_ERROR_CAUSE);
metadata.remove(Constants.STATUS_ERROR_MESSAGE);
metadata.remove(Constants.STATUS_ERROR_SOURCE);
- } else if (status == Status.ERROR) {
+ }
+
+ if (status == Status.ERROR) {
// gone? notify any deleters. Doesn't need to be anchored
collector.emit(Constants.DELETION_STREAM_NAME, new Values(url,
metadata));
+ } else if (status == Status.REDIRECTION && deleteRedirections &&
allowRedirs) {
+ String statusCode = metadata.getFirstValue("fetch.statusCode");
+
+ if ("301".equals(statusCode) || "308".equals(statusCode)) {
Review Comment:
add a comment that we care about PERMANENT redirs
add a method to
https://github.com/apache/stormcrawler/blob/main/core/src/main/java/org/apache/stormcrawler/persistence/Status.java
returning a boolean if the code indicates a permanent redir?
this could be useful in other parts of the project
##########
core/src/main/java/org/apache/stormcrawler/persistence/AbstractStatusUpdaterBolt.java:
##########
@@ -103,6 +109,9 @@ public void prepare(
mdTransfer = MetadataTransfer.getInstance(stormConf);
useCache = ConfUtils.getBoolean(stormConf, useCacheParamName, true);
+ deleteRedirections = ConfUtils.getBoolean(stormConf,
deleteRedirectionsParamName, false);
+
+ allowRedirs = ConfUtils.getBoolean(stormConf,
Constants.AllowRedirParamName, true);
Review Comment:
do we really need to check that redirs have been allowed? they must have
been if redirs are found
##########
core/src/main/java/org/apache/stormcrawler/persistence/AbstractStatusUpdaterBolt.java:
##########
@@ -103,6 +109,9 @@ public void prepare(
mdTransfer = MetadataTransfer.getInstance(stormConf);
useCache = ConfUtils.getBoolean(stormConf, useCacheParamName, true);
+ deleteRedirections = ConfUtils.getBoolean(stormConf,
deleteRedirectionsParamName, false);
+
+ allowRedirs = ConfUtils.getBoolean(stormConf,
Constants.AllowRedirParamName, true);
Review Comment:
this class is quite critical BTW - we want it to remain lean and robust
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
