[GitHub] [nutch] sebastian-nagel opened a new pull request #649: NUTCH-2868 urlnormalizer-protocol fails with StringIndexOutOfBoundsException

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] [nutch] sebastian-nagel opened a new pull request #649: NUTCH-2868 urlnormalizer-protocol fails with StringIndexOutOfBoundsException

GitBox

sebastian-nagel opened a new pull request #649:
URL: https://github.com/apache/nutch/pull/649


   ... when reading invalid line in configuration file
   - log invalid line and skip over it
   - more verbose logging which configuration file is read
   


--
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [nutch] lewismc commented on a change in pull request #649: NUTCH-2868 urlnormalizer-protocol fails with StringIndexOutOfBoundsException

GitBox

lewismc commented on a change in pull request #649:
URL: https://github.com/apache/nutch/pull/649#discussion_r649525019



##########
File path: src/plugin/urlnormalizer-protocol/src/java/org/apache/nutch/net/urlnormalizer/protocol/ProtocolURLNormalizer.java
##########
@@ -82,15 +82,21 @@ private synchronized void readConfiguration(Reader configReader) throws IOExcept
     String line, host;
     String protocol;
     int delimiterIndex;
+    int lineNumber = 0;
 
     while ((line = reader.readLine()) != null) {
+      lineNumber++;
       if (StringUtils.isNotBlank(line) && !line.startsWith("#")) {
         line = line.trim();
         delimiterIndex = line.indexOf(" ");
         // try tabulator
         if (delimiterIndex == -1) {
           delimiterIndex = line.indexOf("\t");
         }
+        if (delimiterIndex == -1) {
+          LOG.warn("Invalid line {} without delimiter: {}", lineNumber, line);

Review comment:
       How exactly do I trigger this warning? Can you provide an example or a trivial unit test?

##########
File path: src/plugin/urlnormalizer-protocol/src/java/org/apache/nutch/net/urlnormalizer/protocol/ProtocolURLNormalizer.java
##########
@@ -177,6 +183,7 @@ public void setConf(Configuration conf) {
       if (reader == null) {
         Path path = new Path(file);
         FileSystem fs = path.getFileSystem(conf);
+        LOG.info("Reading {} rules file {} from {}", pluginName, file, fs);

Review comment:
       Logging the `fs` object results in a log entry
   ```
   2021-06-10 10:51:00,072 INFO  protocol.ProtocolURLNormalizer (ProtocolURLNormalizer.java:setConf(186)) - Reading urlnormalizer-protocol rules file /Users/lmcgibbn/Downloads/nutch/build/urlnormalizer-protocol/test/data/broken_protocols.txt from org.apache.hadoop.fs.LocalFileSystem@e54303
   ```
   Is the `LocalFileSystem@e54303` object reference useful?




--
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [nutch] sebastian-nagel commented on a change in pull request #649: NUTCH-2868 urlnormalizer-protocol fails with StringIndexOutOfBoundsException

GitBox
In reply to this post by GitBox

sebastian-nagel commented on a change in pull request #649:
URL: https://github.com/apache/nutch/pull/649#discussion_r650374772



##########
File path: src/plugin/urlnormalizer-protocol/src/java/org/apache/nutch/net/urlnormalizer/protocol/ProtocolURLNormalizer.java
##########
@@ -177,6 +183,7 @@ public void setConf(Configuration conf) {
       if (reader == null) {
         Path path = new Path(file);
         FileSystem fs = path.getFileSystem(conf);
+        LOG.info("Reading {} rules file {} from {}", pluginName, file, fs);

Review comment:
       Yes, not really useful. Will change.

##########
File path: src/plugin/urlnormalizer-protocol/src/java/org/apache/nutch/net/urlnormalizer/protocol/ProtocolURLNormalizer.java
##########
@@ -82,15 +82,21 @@ private synchronized void readConfiguration(Reader configReader) throws IOExcept
     String line, host;
     String protocol;
     int delimiterIndex;
+    int lineNumber = 0;
 
     while ((line = reader.readLine()) != null) {
+      lineNumber++;
       if (StringUtils.isNotBlank(line) && !line.startsWith("#")) {
         line = line.trim();
         delimiterIndex = line.indexOf(" ");
         // try tabulator
         if (delimiterIndex == -1) {
           delimiterIndex = line.indexOf("\t");
         }
+        if (delimiterIndex == -1) {
+          LOG.warn("Invalid line {} without delimiter: {}", lineNumber, line);

Review comment:
       Seen with a line including no (or an empty) host name. But yes, I'll extend the unit tests.




--
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [nutch] sebastian-nagel commented on pull request #649: NUTCH-2868 urlnormalizer-protocol fails with StringIndexOutOfBoundsException

GitBox
In reply to this post by GitBox

sebastian-nagel commented on pull request #649:
URL: https://github.com/apache/nutch/pull/649#issuecomment-860617706


   Thanks, @lewismc. All points adressed.


--
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [nutch] sebastian-nagel merged pull request #649: NUTCH-2868 urlnormalizer-protocol fails with StringIndexOutOfBoundsException

GitBox
In reply to this post by GitBox

sebastian-nagel merged pull request #649:
URL: https://github.com/apache/nutch/pull/649


   


--
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.

For queries about this service, please contact Infrastructure at:
[hidden email]