[NUTCH-2856] Implement a protocol-smb plugin based on hierynomus/smbj by HiranChaudhuri · Pull Request #826 · apache/nutch

HiranChaudhuri · 2024-10-02T19:56:06Z

[NUTCH-2856] Implement a protocol-smb plugin based on hierynomus/smbj

Draft version of a protocol-smb plugin. Lots of todo comments still,
but it seems to work.

Draft version of a protocol-smb plugin. Lots of todo comments still, but it seems to work.

lewismc · 2024-10-03T04:10:51Z

Moving this to DRAFT status and acknowledging the PR @HiranChaudhuri thank you.
I will try to perform a full review this coming week... thank you.

lewismc

Hi @HiranChaudhuri I added quite a few comments for your consideration. Thanks for submitting this PR 👍
Please ping me once your ready and we can go for round # 2 of peer review.

Further out, I think we could implement some testing for this protocol plugin. We could use testcontainers and essentially spin up a local Samba server using @nddipiazza 's smbj-inttest image. We can come back to this one the PR has evolved a bit.

.gitignore

lewismc · 2024-10-04T15:48:16Z

conf/log4j2.xml

      filePattern="${hadoop.log.dir}/$${date:yyyy-MM}/nutch-%d{yyyy-MM-dd}.log.gz">
-      <PatternLayout pattern="%d %p %c{1.} [%t] %m%n" />
+      <!--<PatternLayout pattern="%d %p %c{1.} [%t] %m%n" />-->
+      <PatternLayout pattern="%d %p %c [%t] %m%n" />


What's the reason for this change? Does this print the logger name in full?

It does print in full as I am otherwise not entirely clear where a message is coming from.
We can revert the change before merge - for the time being on my side I need it.

@HiranChaudhuri please revert. Thank you

runNutch.sh

src/plugin/build.xml

src/plugin/protocol-smb/ivy.xml

src/plugin/protocol-smb/src/java/org/apache/nutch/protocol/smb/Smb.java

src/plugin/protocol-smb/src/java/org/apache/nutch/protocol/smb/SmbURLConnection.java

Improve error handling Rename class as requested Added license header Improve url parsing added robots.txt

lewismc

Thanks for all of the responses @HiranChaudhuri please see my updated suggestions.

.gitignore

lewismc · 2024-10-14T17:52:32Z

conf/log4j2.xml

      filePattern="${hadoop.log.dir}/$${date:yyyy-MM}/nutch-%d{yyyy-MM-dd}.log.gz">
-      <PatternLayout pattern="%d %p %c{1.} [%t] %m%n" />
+      <!--<PatternLayout pattern="%d %p %c{1.} [%t] %m%n" />-->
+      <PatternLayout pattern="%d %p %c [%t] %m%n" />


runNutch.sh

src/plugin/build.xml

src/plugin/protocol-smb/ivy.xml

src/plugin/protocol-smb/plugin.xml

lewismc · 2024-10-16T03:52:38Z

Hi @HiranChaudhuri now that you've activated the plugin test target, I suggest that we implement the unit tests using the test-containers utility. An example can be found at https://java.testcontainers.org/quickstart/junit_4_quickstart/.
For example we could use

@Rule
public GenericContainer redis = new GenericContainer(DockerImageName.parse("ndipiazza/smbj-inttest:1.1"))
    .withExposedPorts(445);

Please let me know your thoughts on this.

HiranChaudhuri · 2024-10-16T18:59:06Z

The container looks good. I have no clue about the @rule annotation and am interested to see how this gets combined together.

lewismc

Trivial header requests. Thanks

conf/url-authentication.xml.template

src/plugin/protocol-smb/src/java/org/apache/nutch/protocol/smb/URLAuthentication.java

…m of content

lewismc

@HiranChaudhuri
Thanks for coming back to this PR, and thanks for the contribution.
Please see my comments, I am happy to work with you in a bid to get this PR into the Nutch codebase.

lewismc · 2025-05-02T20:54:21Z

conf/log4j2.xml

      filePattern="${hadoop.log.dir}/$${date:yyyy-MM}/nutch-%d{yyyy-MM-dd}.log.gz">
-      <PatternLayout pattern="%d %p %c{1.} [%t] %m%n" />
+      <!--<PatternLayout pattern="%d %p %c{1.} [%t] %m%n" />-->
+      <PatternLayout pattern="%d %p %c [%t] %m%n" />


@HiranChaudhuri please revert. Thank you

lewismc · 2025-05-02T20:55:40Z

src/plugin/protocol-smb/ivy.xml

+  <dependencies>
+    <dependency org="com.hierynomus" name="smbj" rev="0.13.0"/>
+<!--
+    These dependencies are either contained in smbj (transitive) or


We can just remove these comments.

lewismc · 2025-05-02T21:01:15Z

src/plugin/protocol-smb/src/test/org/apache/nutch/protocol/smb/TestSmbProtocol.java

+    @Before
+    public void setUp() {
+        LOG.warn("setUp()");
+        Assert.fail();


These tests cannot be committed in the failing state. Thy will destabilize the CI builds.
Previously I suggested using test containers.
If docker is not available on the host machine when tests are being run then we could use the @Testcontainers(disabledWithoutDocker = true) syntax.

[NUTCH-2856] Implement a protocol-smb plugin based on hierynomus/smbj

b568cc9

Draft version of a protocol-smb plugin. Lots of todo comments still, but it seems to work.

lewismc marked this pull request as draft October 3, 2024 04:10

lewismc mentioned this pull request Oct 4, 2024

NUTCH-3073 Address Java compiler warning #828

Merged

lewismc requested changes Oct 4, 2024

View reviewed changes

Reduce logging

a5ad1da

Improve error handling Rename class as requested Added license header Improve url parsing added robots.txt

HiranChaudhuri requested a review from lewismc October 6, 2024 15:59

cube added 2 commits October 7, 2024 11:50

fix connection problems

2d9d4c2

better handling of 'invalid network name' (invalid share names)

3dbb911

lewismc requested changes Oct 14, 2024

View reviewed changes

cube added 2 commits October 14, 2024 22:46

project cleanup

17a9103

remove non-required dependencies

c354088

can use different authentication per URL pattern

a7dc278

lewismc requested changes Oct 16, 2024

View reviewed changes

conf/url-authentication.xml.template Show resolved Hide resolved

src/plugin/protocol-smb/src/java/org/apache/nutch/protocol/smb/URLAuthentication.java Show resolved Hide resolved

cube added 2 commits October 17, 2024 20:39

added ALv2 header

d2726ad

Cache robots.txt to cut network requests in half

379ccf7

HiranChaudhuri marked this pull request as ready for review October 18, 2024 19:40

bugfix: Instread of trunkating at zero bytes by default grab a maximu…

4596f71

…m of content

lewismc requested changes May 2, 2025

View reviewed changes

Comments

Conversation

HiranChaudhuri commented Oct 2, 2024

Uh oh!

lewismc commented Oct 3, 2024

Uh oh!

lewismc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lewismc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lewismc commented Oct 16, 2024

Uh oh!

HiranChaudhuri commented Oct 16, 2024

Uh oh!

lewismc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lewismc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants