Merging final developments into dice-group master by abhihc · Pull Request #97 · dice-group/Squirrel

abhihc · 2019-01-23T22:42:13Z

This pull request is created to merge the functionalities implemented in abhihc/Squirrel to dice-group/Squirrel.

Merging Changes from dice-group/Squirrel to abhihc/Squirrel

Merging changes from Dice-group/Squirrel to abhihc/Squirrel

…s://github.com/varunmaitreya/Squirrel repo to https://github.com/abhihc/Squirrel repository.

…ttps://kanboard.cs.uni-paderborn.de/project/63/task/1295

…edFetcherManager

Merging changes from updated master to develop

Merging changes from develop branch to implementation test scenarios

Getting changes from develop branch into robustness branch

Implementation test scenarios

Merging changes from Robustness branch to Develop branch

Merging develop branch in to master branch

# Conflicts: # spring-config/context-sparql.xml

…/Squirrel

Merging recent changes from robustness branch to master

MichaelRoeder

I left several comments pointing out necessary improvements.

MichaelRoeder · 2019-01-24T12:15:34Z

+		<constructor-arg index="2" value="" />
+	</bean>
+
 </beans>


It looks like you haven't changed the content of the file but the formatting. Changes like that shouldn't be commited at all. Please clarify whether you have changed the files content.

Aakash had made changes to use FileBasedSink for his regular crawling task but I have reverted back the file content similar to that of the dice-group Squirrel.

abhihc@46f55df

Commit ID

The problem is that its formatting is different compared to the "original file". If you want to revert changes, it might be better to check out the "original file" and overwrite your current file with this original file. However, you should check whether this old file really contains all the definitions you need.

OK so the issue has been resolved.

Thanks! That makes it much easier to concentrate on the changes you made 😉

MichaelRoeder · 2019-01-24T12:28:06Z

+    public void fetch() throws Exception {
+        int expected_file_content = 110;
+        HTTPFetcher fetcher = new HTTPFetcher();
+        File data = fetcher.fetch(seeds[0]);


Why is the test generating crawleable URI objects for 4 classes but is checking only one single class?

@MichaelRoeder could you please elaborate on this comment.

The strange name of the entities and property were misleading. I thought you are using four dump files but in the end you have 3 URIs returning the same content. However, this is still confusing. Why do you need all three resources if you are fetching only one of them?

@MichaelRoeder I have removed the unnecessary fetching from two duplicate resources as it returns the same content and testing only one seed and one resource. Have committed the code to Testcase branch in abhihc/Squirrel (abhihc@3640b16). Please let me know if I can push the code to abhihc/squirrel master so that reflects the code changes on this pull request.

That already looks better. However, you added a second fetching step

//checks the crawlable uri object of second class File resourceData1 = fetcher.fetch(new CrawleableUri(new URI(resources[0].getResourceName()))); assertNotNull(resourceData1); assertEquals(expected_file_content,resourceData1.length());

Can you please explain its meaning? What does the test checks here?

Ok. Could you please explain the difference to the first fetching step? Maybe you want to (temporarily) add the following two lines to see what you are fetching:

System.out.println(seeds[0].getUri()); System.out.println(resources[0].getResourceName());

@MichaelRoeder Yes ok. Will add the above two lines to display what I am fetching.

@MichaelRoeder I get "http://127.0.0.1:8089/entity_1.n3" for both the lines. I am fetching from this URI in both the cases.

Exactly. So from my point if view, the second part does not really make sense since you are just repeating what you already did before. 😉

@MichaelRoeder Removed the second part of fetching as it is of not much use.

MichaelRoeder · 2019-01-24T12:29:32Z

+import java.io.File;
+import java.io.IOException;
+
+public class FetcherDummy4Tests implements Fetcher {


Please add javadoc comments to document the aim of this class, its workflow and especially the maining of the pre and post flag attributes.

I would even recommend to rename the pre and post flags since their names are not self explaining.

@MichaelRoeder Successfully added the Javadoc comments and renamed the pre and post flag. Committed these changes to Testcase branch in abhihc/Squirrel (abhihc@a0770cc). Please let me know if I can push the code to abhihc/squirrel master so that reflects the code changes on this pull request.

If you push a commit to the master branch, it will be automatically added to this PR. So no further actions are needed 😉

When you are writing a Javadoc comment, it might be worth to check how it will be shown to other users (your IDE should offer you various ways to see the Javadoc comment of a class). Please note that for example new line characters are removed. In your case, it means that all the text is written directly after each other (which is ok). However, you should make sure that your writing is correct, e.g., that you have ended the sentences with a point. Otherwise, the comment might be confusing for others 😉
(both are just minor comments)

@MichaelRoeder Please let me know if I can insert point and commit again.

@MichaelRoeder Ended the sentences with a point and made it understandable.

MichaelRoeder · 2019-01-24T12:35:28Z

+<http://dbpedia.org/resource/Tonawanda_(town),_New_York> <http://www.w3.org/2002/07/owl#differentFrom> <http://dbpedia.org/resource/New_York> .
+<http://dbpedia.org/resource/Ethan_A._Hitchcock_(general)> <http://dbpedia.org/ontology/deathPlace> <http://dbpedia.org/resource/New_York> .
+<http://dbpedia.org/resource/Greg_Gutfeld> <http://dbpedia.org/ontology/residence> <http://dbpedia.org/resource/New_York> .
+<http://dbpedia.org/resource/BDMI> <http://dbpedia.org/ontology/location> <http://dbpedia.org/resource/New_York> .


Test cases typically aim at certain situations. This looks like a simple copy from DBpedia. Although it does the job, it is better to a) reduce the number of triples to a number that a human can overlook fast and b) make sure that this small number covers all important cases (e.g., the usage of blank nodes or literals seems to be missing in this file).

I have reduced the number of triples. I have added couple of faulty files to cross check the automata which cover the conditions like faulty triples and wrong syntaxes of the file.

Nice. Could you please make sure that you have a test in which you have literals as objects? It looks like this important part is missing in the current test files.

Do you been mean the files containing the test cases?

Or the sample test files used as input to check the automata?

Yes. Your current test files are focusing on URI resources as subject and object. Typically, you would also have examples with literals as objects and blank nodes as subjects and objects.
Unfortunately, these missing tests led to the problem described in the comment in the FiniteStateMachineFactory.java class.

Ok, so the Automata's written are completely focused on triple which has the subject, object, and predicate in URI format. I focused completely on the resources which were available in the project.

… flag

…hod to closeSinkforURI method FrontierImp: changed the LOGGER.debug back to LOGGER.info when URI is good

Getting changes from master to develop

develop to robustness

Merging changes from robustness into develop

Merging changes from develop to Testcase

Merging recent changes from Testcase branch to develop branch

Final changes from develop to master

MichaelRoeder · 2019-01-25T11:25:48Z

+     * NTRIPLES file
+     */
+
+    private FiniteStateMachine buildNTriplesStateMachine() {


@abhihc
As we discussed before during on of our meetings, having an N-triples parser is not necessary since N-triple is a subset of turtle (see https://www.w3.org/TR/n-triples/#sec-n-triples-language). So N-triple should have been covered by your turtle parser. Unfortunately, your turtle parser is only looking at the keyword @prefix (again, something that we discussed early on and warned you that this won't work).

However, the main issue with both set of rules is that they are not able to detect that the following line is a valid N-triples file:

_:alice <http://xmlns.com/foaf/0.1/knows> _:bob . _:bob <http://xmlns.com/foaf/0.1/knows> _:alice .

See https://www.w3.org/TR/n-triples/#BNodes

Ok, so the Automata's written are completely focused on triple which has the subject, object, and predicate in URI format. I focused completely on the resources which were available in the project.

But where is the sense in stating that we only crawl resources that we already have in our project? 😕

I asked you to have a look at the specifications of the languages and after we discussed the first ideas that you had regarding this task, I asked you again to take a look at them, e.g., to make sure that comment lines do not create issues, etc. We also discussed that your first test cases where not sufficient since you had only one simple example for each language and I asked you to add more test cases to make sure that you cover the complete language.

However, since today is the deadline for the code, I do not see a possibility to fix these issues in the remaining time 😞

Regarding the comment lines issue, I have added the condition in the MimeTypeDetector class. But yes adding the conditions to check triple which are of other formats than URI would take time. Anyways Thanks for all the inputs.

I was already wondering, how this "magic" works 😄

Unfortunately, adding this to the MimeTypeDetector class has two disadvantages.

You are splitting up the definition of the languages. One part of the languages is now part of the automatons while another part is in the MimeTypeDetector class.

You are defining lines starting with # as general comment lines. However, this is not true for all the languages, e.g., XML. You could easily imagine a situation in which this can lead to a large amount of issues.

Yes, I get it. I overlooked it in a generic manner.

…d the repeated fetching in HttpFetcherTest

Test scenarios to develop

Develop to master

Getting changes from Testscenarios branch to develop

Merging changes from develop to master

master to develop

develop to master

abhihc and others added 24 commits October 24, 2018 11:26

Merge pull request #1 from dice-group/master

b8a863f

Merging Changes from dice-group/Squirrel to abhihc/Squirrel

Merge pull request #2 from dice-group/master

3fcc30d

Merging changes from Dice-group/Squirrel to abhihc/Squirrel

Implemented junit test for WorkerImpl to test the worker

4ee15af

Manual transfer of code of mime type detector functionality from http…

961d052

…s://github.com/varunmaitreya/Squirrel repo to https://github.com/abhihc/Squirrel repository.

Adding Javadoc comments.

a4cb0a9

Changes made for collecting performance data for the crawler. Task: h…

46f55df

…ttps://kanboard.cs.uni-paderborn.de/project/63/task/1295

Testcases for ZipArchiver, SqlBasedIterator, HttpFetcher, SimpleOrder…

b4c676a

…edFetcherManager

Merge pull request #6 from abhihc/master

3026e28

Merging changes from updated master to develop

Merge pull request #7 from abhihc/develop

13ab5d9

Merging changes from develop branch to implementation test scenarios

Merge pull request #8 from abhihc/develop

b9feb40

Getting changes from develop branch into robustness branch

Merge pull request #10 from abhihc/Implementation_test_scenarios

cb66638

Implementation test scenarios

Merge pull request #9 from abhihc/Robustness

050d69a

Merging changes from Robustness branch to Develop branch

Merge pull request #11 from abhihc/develop

2a0934a

Merging develop branch in to master branch

Changes made in context-sparql.xml file according to dice-group/Squirrel

3564036

Merge remote-tracking branch 'origin/master'

9c42ea2

# Conflicts: # spring-config/context-sparql.xml

Final Changes made in context-sparql.xml file according to dice-group…

b9f78d0

…/Squirrel

Merging changes from dice_group/master

b3f01fa

Merge branch 'dice-group-master'

397221f

Fixed null pointer execption.

afbab67

Fixed null pointer execption.

b01bd05

Fixed null pointer execption.

20a2f97

Fixed null pointer execption.

fb40474

Fixed assertion error

a9b5f96

Merge pull request #14 from abhihc/Robustness

ade55b1

Merging recent changes from robustness branch to master

MichaelRoeder changed the base branch from master to developForMerging2 January 24, 2019 12:08

MichaelRoeder requested changes Jan 24, 2019

View reviewed changes

Ajaykps and others added 4 commits January 24, 2019 15:59

Added JavaDoc comments to FetcherDummy4Tests and renamed pre and post…

a0770cc

… flag

Changes made according to the pull request : dice-group#97

3f2bc68

FilebasedSink: Moved the debugging of triple count from addTriple met…

eb46c36

…hod to closeSinkforURI method FrontierImp: changed the LOGGER.debug back to LOGGER.info when URI is good

Testing all the generated crawlable URI objects

3640b16

ajrox090 and others added 7 commits January 24, 2019 17:20

commented PerformanceAnalysis debug because of travis build failure

f10be5d

Merge pull request #19 from abhihc/master

4a4f6de

Getting changes from master to develop

Merge pull request #20 from abhihc/develop

4fd4ed8

develop to robustness

Merge pull request #21 from abhihc/Robustness

fae2f2e

Merging changes from robustness into develop

Merge pull request #22 from abhihc/develop

9344b9d

Merging changes from develop to Testcase

Merge pull request #23 from abhihc/Testcase

f8e216a

Merging recent changes from Testcase branch to develop branch

Merge pull request #24 from abhihc/develop

560ccb4

Final changes from develop to master

MichaelRoeder reviewed Jan 25, 2019

View reviewed changes

abhihc and others added 13 commits January 25, 2019 12:43

Restored the orginal similar to DICE group

aee8e4b

Merge remote-tracking branch 'origin/master'

3421990

Ended with point in javadoc comments in FetcherDummy4Tests and remove…

5833cd3

…d the repeated fetching in HttpFetcherTest

Removed extra space in formatting

8868f0a

Merge pull request #26 from abhihc/TestScenarios

da0bdb9

Test scenarios to develop

Merge pull request #27 from abhihc/develop

fc5ed1b

Develop to master

Changes made to fix the issues got from codacy

7574c2a

Changes made to fix the issues of codacy build

7dc3e53

Changes made to fix the issues of codacy build

d350b40

Merge pull request #29 from abhihc/TestScenarios

8b3d87a

Getting changes from Testscenarios branch to develop

Merge pull request #30 from abhihc/develop

0ce8f85

Merging changes from develop to master

Merge pull request #31 from abhihc/master

8d0d447

master to develop

Merge pull request #32 from abhihc/develop

03d9eb5

develop to master

MichaelRoeder merged commit 97801ab into dice-group:developForMerging2 Jun 11, 2019

Uh oh!

Conversation

abhihc commented Jan 23, 2019

Uh oh!

MichaelRoeder 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!

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!

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!

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!

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!

MichaelRoeder Jan 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaelRoeder Jan 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

MichaelRoeder Jan 25, 2019 •

edited

Loading

MichaelRoeder Jan 25, 2019 •

edited

Loading