Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace deprecated routines in guava #50

Merged
merged 18 commits into from
Mar 18, 2014

Conversation

shriphani
Copy link
Contributor

No description provided.

@nlevitt
Copy link
Contributor

nlevitt commented Mar 13, 2014

Thanks for the contribution!

Two things though:

Related: https://webarchive.jira.com/browse/HER-2033

@shriphani
Copy link
Contributor Author

Whoops, sorry about that. Fixed.

@shriphani
Copy link
Contributor Author

actually a couple more issues emerged. I found a few more deprecated calls. Give me a couple of mins and I'll have it ready

@shriphani
Copy link
Contributor Author

ok i can execute a crawl with this version

@nlevitt
Copy link
Contributor

nlevitt commented Mar 14, 2014

I checked out your fork, merged from origin/master, ran "mvn install" and got compile errors...

[ERROR] /home/nlevitt/workspace/heritrix3/modules/src/main/java/org/archive/modules/forms/FormLoginProcessor.java:[123,26] error: <K,V>makeComputingMap(Function<? super K,? extends V>) is not public in MapMaker; cannot be accessed from outside package
[ERROR]
K extends Object declared in method <K,V>makeComputingMap(Function<? super K,? extends V>)
V extends Object declared in method <K,V>makeComputingMap(Function<? super K,? extends V>)
/home/nlevitt/workspace/heritrix3/modules/src/main/java/org/archive/modules/forms/FormLoginProcessor.java:[132,26] error: <K,V>makeComputingMap(Function<? super K,? extends V>) is not public in MapMaker; cannot be accessed from outside package
[ERROR]
K extends Object declared in method <K,V>makeComputingMap(Function<? super K,? extends V>)
V extends Object declared in method <K,V>makeComputingMap(Function<? super K,? extends V>)
/home/nlevitt/workspace/heritrix3/modules/src/main/java/org/archive/modules/fetcher/FetchWhois.java:[451,30] error: cannot find symbol
[ERROR] class InternetDomainName
/home/nlevitt/workspace/heritrix3/modules/src/main/java/org/archive/modules/fetcher/FetchWhois.java:[454,59] error: cannot find symbol

@shriphani
Copy link
Contributor Author

Hi, one of the included jars (containing org.archive.util.TextUtils) is making a call to (the deprecated) MapMaker and this causes a bunch of tests to fail and the crawler to not be instantiated. I am guessing that the culprit is sitting in the archive-commons repo since the TextUtils source isn't included in heritrix.

@nlevitt
Copy link
Contributor

nlevitt commented Mar 14, 2014

Looks like you're right. webarchive-commons pulls in guava 14.0.1 which is about a year old. (Much newer than r08 that heritrix depends on.) So probably the thing to do is take the explicit guava dependency out of heritrix pom.xml and let it inherit from webarchive-commons. A lower priority would be updating the webarchive-commons dependency to 16.0.1. Either way some heritrix code will have to change, and possibly some webarchive-commons code as well.

Are you up for that? Btw heritrix uses this fork https://github.com/internetarchive/webarchive-commons so any pull request should target that. We'll get the changes merged upstream later.

@shriphani
Copy link
Contributor Author

I can take a look at it.

@shriphani
Copy link
Contributor Author

Also, all the tests don't pass on the master branch. I cloned internetarchive/heritrix and 2 tests failed:

https://gist.github.com/shriphani/9554002

I can't seem to be able to create an issue on github or on jira so I pasted it here.

@shriphani
Copy link
Contributor Author

Ok, try now:

This is the output of mvn install on my machine. The one test that fails is testReadConsistencyUnderLoad(org.archive.util.ObjectIdentityBdbManualCacheTest) but from my experience. Do you know if this is related to the guava bump ?
https://gist.github.com/9555502

@shriphani
Copy link
Contributor Author

Ok, I can get heritrix to build and I have been able to crawl as well.

Here's the output of mvn install and mvn -DskipTests install on the current build:

https://gist.github.com/shriphani/9560441

@nlevitt
Copy link
Contributor

nlevitt commented Mar 17, 2014

Thanks!

I got compile errors which I fixed with this small change:

diff --git a/modules/src/main/java/org/archive/modules/forms/FormLoginProcessor.java b/modules/src/main/java/org/archive/modules/forms/FormLoginProcessor.java
index 47d06ce..aa537d2 100644
--- a/modules/src/main/java/org/archive/modules/forms/FormLoginProcessor.java
+++ b/modules/src/main/java/org/archive/modules/forms/FormLoginProcessor.java
@@ -121,7 +121,7 @@ public class FormLoginProcessor extends Processor implements Checkpointable {
     // formProvince (String) -> count
     ConcurrentMap<String, AtomicLong> eligibleFormsSeenCount =
             CacheBuilder.newBuilder()
-                .build(
+                .<String, AtomicLong>build(
                     new CacheLoader<String, AtomicLong>() {
                         public AtomicLong load(String arg0) {
                             return new AtomicLong(0L);
@@ -131,7 +131,7 @@ public class FormLoginProcessor extends Processor implements Checkpointable {
     // formProvince (String) -> count
     ConcurrentMap<String, AtomicLong> eligibleFormsAttemptsCount =
             CacheBuilder.newBuilder()
-                    .build(
+                    .<String, AtomicLong>build(
                             new CacheLoader<String, AtomicLong>() {
                                 public AtomicLong load(String arg0) {
                                     return new AtomicLong(0L);

Could you incorporate that into your pull request?

And finally could you merge the latest from origin/master and sanity check once more?

@shriphani
Copy link
Contributor Author

Done.

I can build and crawl. Here's the output (with and without tests): https://gist.github.com/shriphani/9592177

@shriphani
Copy link
Contributor Author

Alright, I've pulled, rebased and all that. Let me know if something else is off.

nlevitt added a commit that referenced this pull request Mar 18, 2014
Replace deprecated routines in guava
@nlevitt nlevitt merged commit ef2889c into internetarchive:master Mar 18, 2014
@nlevitt
Copy link
Contributor

nlevitt commented Mar 18, 2014

I can build and crawl. Here's the output (with and without tests):
https://gist.github.com/shriphani/9592177

Hmm your gist says tests failed...
Failed tests: testSerializationIfAppropriate(org.archive.modules.credential.CredentialStoreTest)
testSerializationIfAppropriate(org.archive.modules.credential.HtmlFormCredentialTest)

But the tests pass for me. I'll merge now.

@shriphani
Copy link
Contributor Author

Woot! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants