Increasing poling interval to 30 sec

[daniyarchambylov]

On a daily bug review call we we discussed https://diligence.atlassian.net/browse/MBT-33 and from discussion with @rocky today I believe that we are all agreed on setting polling interval to 30 seconds because analysis on the simplest contract takes up to 30 seconds

[coveralls]

Pull Request Test Coverage Report for Build 293

• 19 of 19 (100.0%) changed or added relevant lines in 3 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+3.7%) to 85.75%

---

Totals
Change from base Build 275:	3.7%
Covered Lines:	238
Relevant Lines:	274

---

💛 - Coveralls

[daniyarchambylov]

My bad @fgimenez @rocky

[rocky]

AIUI we should wait initially 30sec and then start polling with the schema we agree on (currently constant step == 1sec), but the step itself should not be 30sec, is that correct @rocky?

Here is my understanding. If a request is cached we get "FInished" in the response to the initial v1/analyses request. Right @birdofpreyru? If it is not cached or we don't get a "Finished" in response to the v1/analyses then yes we wait that amount of time before the first poll and only that poll.

What happens next is not in this PR but the next one after that, so I'm not going to sweat what the polling delay after the first (long) one.

The next PR is about doing an exponential delay. Let's say we want no request to ever poll more than 10 times. (You all tell me what the max poll count should be).

Given that we always have a maximum wait time, we use that to calculate the base of the exponent used in the exponential delay.

[fgimenez]

Thanks @rocky makes sense :+1:

[birdofpreyru]

@rocky correct; and it looks to me it is not what the current code in this PR does: it will always wait 30 seconds before trying to get results, it does not check whether the initial POST /v1/analyses returned response with status: "Finished", indicating that results were copied from a previous analysis (cache hit).

[rocky]

@rocky correct; and it looks to me it is not what the current code in this PR does: it will always wait 30 seconds before trying to get results, it does not check whether the initial POST /v1/analyses returned response with status: "Finished", indicating that results were copied from a previous analysis (cache hit).

@birdofpreyru Thanks for information. I have been so focused with other stuff that I hadn't had time to look at this in detail. Since you are the protocol guy here, please review ec5ba74 to make sure it gets closer. There may be an additional probe for status even though we detect the job is Finished, but I spent way too much time today that I don't have unsuccessfully trying to figure out how to eliminate that. Since it is one or two extra probes, right now I am not sure it is worth the effort.

A lot of the unit tests however had to be commented out since we are now returning the entire response data rather than just the uuid as we previously did. @daniyarchambylov perhaps you can fix the unit tests up. Again, I spent too time trying to figure out how to adjust the data structures in the tests, to no avail.

Finally, I'll say that my interest here isn't academic. I need at least this to be able to start going through aragonOS which is what I really want to focus on.

[birdofpreyru]

I did not test the code in action; but just looking at it, it seems it will work fine as is, though I suggested a few improvements above.

[rocky]

@birdofpreyru avows:

I did not test the code in action

No problem. I will make the suggested changes and do testing since I need to use this code anyway. Thanks for looking this over and giving the thoughtful great suggestions.

[rocky]

Has been incorprorated into and superceded by geometric-delay branch