Review (NS-145): Assignment B.2.2b: Make bitarchive replica use new messages.

Author

Jonas

Moderator

Jonas

State

Review

Objectives

See FR 1823.

Total Time Used (Coding,Documentation,Review):

Time use (Coding,Documentation,Review)
SVC: 1.0
JOLF: 5.0

General comments:

Description

Classification

Status

Use different termilogy: wrong/bad => corrupt

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/checksum/distribute/ChecksumFileServer.java', revision 1246

Lines

Description

Classification

Status

263

Should we validate the badFile before sending it back?

Cosmetic

REJECT

Comments on file 'trunk/src/dk/netarkivet/archive/arcrepository/distribute/JMSArcRepositoryClient.java', revision 1246

Lines

Description

Classification

Status

684

Replace 'wrong' with 'corrupt'

Cosmetic

OK

717, 721

This should be 'removedFile' instead of retrieving the variable again.

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/checksum/distribute/CorrectMessage.java', revision 1246

Lines

Description

Classification

Status

79

"String checksum" => "String badChecksum"

Cosmetic

OK

173-176

Issue warning, and return null.

Minor

OK

Comments on file 'trunk/src/dk/netarkivet/archive/checksum/FileChecksumArchive.java', revision 1246

Lines

Description

Classification

Status

723-724

Language: Change to "Make the file containing the corrupt entry be returned in the CorrectMessage

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/arcrepository/bitpreservation/DatabaseBasedActiveBitPreservation.java', revision 1254

Lines

Description

Classification

Status

85

Remove. This will not be implemented.

Cosmetic

OK

138-139

Split up into multiple commands

Cosmetic

OK

159-160

Split up into multiple commands

Cosmetic

OK

446

Describe, when this will be implemented

Cosmetic

OK

469

Describe, when this will be implemented

Cosmetic

OK

515, 519

Does not throw an UnknownID (or IOFailure? - perhaps indirectly)

Cosmetic

OK

601

Add when this will be implemented

Cosmetic

OK

641

typo: teh => the

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/arcrepository/ArcRepository.java', revision 1242

Lines

Description

Classification

Status

358-359

Change to "Checksum job message submitted for file '" + filename + "' with message id '" + msg.getID + "'

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/checksum/ChecksumArchive.java', revision 1246

Lines

Description

Classification

Status

72

Does "incorrectChecksum" refer to an undefined variable?

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/bitarchive/distribute/RemoveAndGetFileMessage.java', revision 1246

Lines

Description

Classification

Status

60-61

What prevents you from removing this constructor, now?

Cosmetic

OK

98

Typo: "fopr .." => "for the file"

Cosmetic

OK

101-102

Add missing argument-check

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/checksum/distribute/ChecksumClient.java', revision 1247

Lines

Description

Classification

Status

66

change "theCR" to "theChecksumChannel" or "theChecksumQueue"

Cosmetic

OK

192

What is this TODO about? I don't understand it

Cosmetic

OK

192

Remove! The todo is fixed

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/bitarchive/distribute/BitarchiveMonitorServer.java', revision 1251

Lines

Description

Classification

Status

107, 110, 116

"First" => "In the first stage" "Second" => In the second stage" "Third" => "In the third stage"

Cosmetic

OK

112

an UploadMessage

Cosmetic

OK

240-251

This description here is redundant

Cosmetic

OK

258

Typo: Recieving => Receiving

Cosmetic

OK

258, 298-299

Receiving

Cosmetic

OK

280-281

Redundant description

Cosmetic

OK

289-291

Redundant description

Cosmetic

OK

308

Typo: filed => failed

Cosmetic

OK

333-340

redundant description

Cosmetic

OK

368, 371

Finish the two sentences.

NA

OK

373

argument validation

Cosmetic

OK

389

argument validation + javadoc

Cosmetic

OK

405

argument validation + javadoc.

Cosmetic

OK

444

Trouble while handling batch request => Unable to handle batch request due to unexpected exception

Cosmetic

OK

561

Which other exceptions are foreseen here besides the IOFailure thrown in case of unhandled message type? The called method handle their own exceptions, so none. The 'try - catch' is redundant if the reply is put into the last 'else' clause.

Cosmetic

OK

640

Change to: Fetch the contents of the batchresultfile

Cosmetic

OK

657

Include the others in the log entry

Cosmetic

OK

664

explain what valid means here

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/arcrepository/bitpreservation/FileBasedActiveBitPreservation.java', revision 1254

Lines

Description

Classification

Status

296

This method is redundant: the method only forwards to the getChecksum method

Cosmetic

OK

347

Wrong spelling of 'successful'

Cosmetic

OK

363

Wrong checkNotNull message

Cosmetic

OK

402

Replace log message with "Finding missing files in directory '"

Cosmetic

OK

413

Add to comment what difference set 1 is

Cosmetic

OK

431

Add to comment what difference set 2 is

Cosmetic

OK

449

Replace log message with "Finished finding missing files"

Cosmetic

OK

472-474

Make an external variable for value of "ArcRepositoryClientFactory.getPreservationInstance().getAllFilenames(replica.getId))" so this value can be checked before calling the copyFile method

Cosmetic

OK

478

Don't use the word 'wrong'. Use the word 'corrupt' instead.

Cosmetic

OK

619-620

Make an external variable for value of "ArcRepositoryClientFactory.getPreservationInstance().getAllChecksums(replica.getId))" so this value can be checked before calling the copyFile method

Cosmetic

OK

718-720

Please change to: Check that the files we want to restore are indeed missing on the replica on which we are restoring them, and presnt in admin data and the reference replica. If so, upload the missing files from the reference replica to this replica.

Cosmetic

OK

873

Add that: We don't wait for an acknowledgement that admin data has indeed been updated.

Cosmetic

OK

916

change "a bad entry" to "an entry"

Cosmetic

OK

962

When will this be implemented?

Cosmetic

OK

975

When will this be implemented?

Cosmetic

OK

988

Change to "String... filenames"

Cosmetic

OK

990

When will this be implemented?

Cosmetic

OK

1023

When will this be implemented?

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/bitarchive/distribute/BitarchiveClient.java', revision 1247

Lines

Description

Classification

Status

229-230

Remove this @throws clause

Cosmetic

OK

243

why was this line added?

Cosmetic

OK

247

Typo: be send => be sent

Cosmetic

OK

286-288

Document that a Argument not valid is thrown

Cosmetic

OK

313

remove

Cosmetic

OK

IssuesFromNs145 (last edited 2010-08-16 10:24:44 by localhost)