Code refactor for router

[vkuznet]

Rishi, please, please test your code BEFORE you submit PR. I already provided a test/run.sh script which does everything. Just use it. I tried to apply your PR and it fails.

test/run.sh pull dataset

....

# I got the following:
Main agent REQUEST table
1|36ba8f8d-c9b2-5115-a3c7-b9e505c2616c|||/a/b/c|http://localhost:8000|sourceAgent|http://localhost:9000|destinationAgent|http://localhost:8989|mainAgent|pending|0

Source agent tables
Files table:
1|file.root|/tmp/transfer2go/source/file.root|1|1|1048576|00f00001|0|0
2|file2.root|/tmp/transfer2go/source/file2.root|2|1|1048576|00f00001|0|0
Blocks table:
1|/a/b/c#123|1
2|/a/b/c#987|1
Datasets table:
1|/a/b/c

Destination agent tables
Files table:
1|file2.root|/tmp/transfer2go/destination/file2.root|1|1|1048576|00f00001|0|1507987287
2|file.root|/tmp/transfer2go/destination/file.root|1|1|1048576|00f00001|0|1507987287
Blocks table:
1||1
Datasets table:
1|

Test failed

It means that your code only transferred files, but it didn't updated properly meta-data, see that block and dataset tables are empty at destination.

The problem seems to be at line 225 of core/request.go where you mixed logic in if statement. Removing that line will fix test/run.sh test.

Please apply requested changes, TEST YOURSELF that it works with the following commands:

# test pull model
test/run.sh pull dataset
test/run.sh pull block
test/run.sh pull file

# test push model
test/run.sh push dataset
test/run.sh push block
test/run.sh push file

and then let me know that I can review this PR again.

Once it is working, I can later take care of code-refactoring.

You may also try to change test/run.sh (see where config files are created) and change router usage. Ultimately, we need to extend test/run.sh to accept third parameter to accept usage of router, e.g. test/run.sh pull dataset router will enable router, while test/run.sh pull dataset will run without router (right now router is enabled by default). And, later we can extend test/run.sh to setup another source to provide tests to transfer partial dataset via multiple sources.

[rishiloyola]

Hi Valentin, I submitted this request after testing it properly. But somehow I forgot to check whether it is correctly updating the destination's catalog or not.

It is not filling up information related to blocks and files simply because of missing dataset and block parameter's value over here. link I need to pass their values too instead of just passing the file details.

It is successfully passing all the tests for the push model. Proof - link

[vkuznet]

Rishi, test failed period. It means that you didn't test it fully. We need to support both models, so test should pass for both of them.

Now, I provided you the pointer to wrong if/else if/else statement. It mixed two different conditions, the RouterModel and TransferType. You CANNOT put them in a single if/else if/else statment, it is plain wrong. You may have if RouterMode block where you can check TransferType but you can't mix them up in a single if/else if/else statement.

I don't know if code will work if you'll fill out missing block/dataset info. But as I said you MUST resolve all parameters in RequestTransfer before passing it around to src/dst agents. Agents should just take request and perform action. The request should contains all info about files, blocks, dataset, agent URLs and its aliases, etc.

Let me know when you'll fix all problem that I can review the PR.

[rishiloyola]

Regarding if/else statement,

The logic is

if routerModel == true {
  - Resolve request through router for both the model (push or pull model)  // roter = true, model = push or pull
} else {
  if (model == "pull") {
    - resolve request and send it to destination  // router = false, model = pull
  } else {
    - Its push model do not resolve request directly send it to the source.  // router = false, model = push
  }
}

Instead of writing this way I just combined else with if. If you think this can be the flaw or bad coding style then let me know I will shift that condition in else block instead of writing else if model == "pull".

[rishiloyola]

@vkuznet Tests are now working fine for me. Can you review this? The test was failing because of the missing parameters. Check out my last commit. I fixed that problem.

• Test result for the push model - gist
• Test result for the pull model - gist

[vkuznet]

ok, code works now. Please do few things before the merge:

• change if/else if/else statement as you replied, this structure looks and reads (!) much better. Moreover add comments as you shown in your reply.
• please revert changes to json files, as I wrote I don't want to have config point to the code area itself, everything should points to /tmp area. After that I can start doing code re-factoring and clean-up.

[rishiloyola]

@vkuznet made necessary changes. You can now merge this.