Task state set incorrectly when best TES instance fails about protes HOT 4 CLOSED

Yep @uniqueg
Yours is much better solution and I will implement it.
While going through the code-base, I found some typos as well. I will raise a single issue related to all these typos soon.

from protes.

Hi @Ayush5120 ,
I would love to solve this issue. After carefully examining the code base in this repository, I have identified the cause of the issue as being the following code block: link.

I can provide a simple version of a similar code block that I created, which generates a divide by zero exception at i == 0 but still allows the loop to continue. This will help to explain the reason behind the issue."

try:
    x = 0 
    for i in range(0, 10):
        try:
      	    x = 10 / i;
        except:
      	    print('SYSTEM ERROR: Can\'t divide by zero')
        print(x)
        continue
except:
    print("Error");

The o/p will be:

SYSTEM ERROR: Can\'t divide by zero
0
10.0
5.0
3.3333333333333335
2.5
2.0
1.6666666666666667
1.4285714285714286
1.25
1.1111111111111112

This scenario is comparable to our case, in which if TES fails at any point (similar to dividing by 0 when i == 0), it sets the state to 'SYSTEM ERROR' and continues searching for the next best TES.

A simple solution to address this issue could be to store the current index in a variable and, in the exception block, add a check to see if index == len(tes_uri_list) - 1. If this condition is true, set the state to 'System Error'; otherwise, continue searching for the next TES.

Ex:

try:
    l = 9
    x = 0 
    for i in range(0, 10):
        try:
      	    x = 10 / i;
        except:
            if l == i:
      	        print('SYSTEM ERROR')
            else:
      	        print('Skipping for next TES')
        print(x)
        continue
except:
    print("Error");

which gives output as:

Skipping for next TES
0
10.0
5.0
3.3333333333333335
2.5
2.0
1.6666666666666667
1.4285714285714286
1.25
1.1111111111111112

from protes.

Thanks @SohamRatnaparkhi, I have assigned you to the issue.

However, one suggestion: Instead of keeping an index variable and checking whether you are in the last iteration of the loop - why not just let the loop conclude and only update the state in the end? That way, all TES instances (first or last) are treated equally (log messages will be printed indicating why a given TES instance was dropped) and then in the end - if you're still around after the loop (which means you never hit the return) you can just log that all known TES instances were dropped (the individual reasons can then be found further up in the logs) and that only then you set the state to SYSTEM_ERROR.

This will also lead to less code rather than more.

BTW, I see a whole lot of issues with the .create_task() method. Try not to get sidetracked by them an focus on just fixing this one. We will get to the others on time. However, feel free to raise any issues that you see with the code :)

from protes.

Closed by #143

from protes.