Modified setup_dirac_proxy time parsing to allow for proxies with time greater than 24 hours

[philiptgrace]

I initialised a proxy for 72 hours, but the time parsing failed, since setup_dirac_proxy tried to match a string like '71:42:32' to '%H:%M:%S'.

As far as I know, there is no strptime alternative to %H to accept numbers greater than 23. In this PR, I split the time string and pass each component to timedelta, which resolves the issue.

[meliache]

Thanks for finding and figuring out this bug. I thought that running gb2_proxy_init only initializes the dirac proxy for 24 hours and didn't know that 72 hours are possible.

Context for any other potential reviewers: Among other things, gb2_proxy_info returns for how much time the dirac proxy is still valid. I actually only care if that time string is 00:00:00, then I want to re-initialize the proxy, otherwise it isn't re-initialized, so that the user isn't asked for the password more times than necessary. Now the question is, why didn't I just check for

if timeleft_str == "00:00:00"

I just wanted to make sure that timeleft_str is a time string and I wanted to get an error if due to unforeseen reasons (we are doing dirty string parsing after all) timeleft_str is something else. This was the major reason to applying strftime. Basically, your code should also raise an error if the string that we parsed is not a string of 3 integers separated by colons. I'm just wondering myself we could simplify this further...

The optimal way would be to remove all the string parsing and write our own python2 script that interacts with gbasf2/dirac and has a return value depending on whether the proxy is still and run that as a subprocess. If there isn't already a script for that we aren't aware of.

But that shouldn't be our worry in this PR, on a first glance this looks really good and I will look at the code of the whole function again after having slept out.

[philiptgrace]

The -v/--valid option accepts a HH:MM value to set the expiry time. In fact, this might be a useful setting to implement in b2luigi.

I agree that a python2 script interfacing directly with gbasf2/dirac is the ultimate solution to this. This snippet of code retrieves the time left (in seconds) if a proxy is active. I haven't tested its behaviour if a proxy is not active. Judging by how getProxyInfo is treated elsewhere in the code, it seems to raise a KeyError if no proxy is active.

from BelleDIRAC.gbasf2.lib.auth import getProxyInfo

ProxyInfo = getProxyInfo()
seconds = ProxyInfo["Value"]["secondsLeft"]

EDIT: I just checked the behaviour of getProxyInfo when no proxy is active, and it results in a sys.exit, which is annoying.

[meliache]

Cool that you found how to do it. We can use that it exits with an exit code of
1 when called before the proxy had been initialized. I would append to your
snippet the lines

if seconds == 0:
    sys.exit(1)

and put that script into

b2luigi/batch/processes/gbasf2_utils/

Then, we can call that in a subprocess and if the return code is 0 (successful
error code, meaning seconds > 0), do nothing, but if it returns with an error
code, run gb2_proxy_init -g belle. I'll try that out on a new branch.

I'm surprised that the option -v HH:00 takes hour values larger than 24. Is 72
the maximum or can you make it indefinitely large?

[meliache]

I just created a new branch and implemented this idea. For the code you can look here:

• gbasf2_utils/check_if_dirac_proxy_is_initialized.py
• new setup_dirac_proxy()

I also have a commit in there which improves how this method is called during execution, namely as a side effect of run_with_gbasf(). I'm thinking of creating a new PR for that, but maybe we can still first merge yours, since it has less changes to the code and I'm sure this works (you tested it, right?).

[philiptgrace]

Is 72 the maximum or can you make it indefinitely large?

I just tested this, and found the maximum length I could initialise a proxy for was 136 days, which is the same amount of time my VOMS AUP signature is valid for. So I figure that is the upper limit. (The command will succeed if you pass a much larger number, but will set the time to the maximum value.)

(you tested it, right?)

Yes. Tested the snippet in isolation, and also as a part of job submission.

I read the code on your new branch, and it all looks good to me. Two questions/comments:

• How much manual parsing of command-line output exists in this gbasf2 wrapper, and how much of it do you think could be replaced by similar Python2 code like this?

• In addition to having a user option called something like gb2_proxy_time (to be passed to the -v flag), it may be useful to have an option gb2_proxy_minimum_time (defaulting to 0), to re-initialise the proxy if the time left is less than the given number of hours. The usecase I am imagining is a long submission/monitoring/downloading time where the user doesn't want their proxy to timeout part way through.

  The implementation would be simple: pass the gb2_proxy_minimum_time setting to check_if_dirac_proxy_is_initialized.py as an argument, and change the time check to if seconds > sys.argv[1] * 60: sys.exit(0).

[meliache]

How much manual parsing of command-line output exists in this gbasf2 wrapper, and how much of it do you think could be replaced by similar Python2 code like this?

So far I had only written a python2 script like this for obtaining a json with all the job statuses, since this is the core part of my wrapper and there I had had bugs before that. Why I haven't replaced everything yet was laziness and I don't like working with gbasf2 code, since I can't just pip-install it on my notebook and then all the nice IDE features I have hear like "jump to definition" etc. don't work, so finding things in the BelleDirac code is harder for me.

I opened a new issue for that with all instances where I think I could replace this with similar code here: #47

Also, I opened an issue for your suggestions regarding the proxy time: #48

[nils-braun]

Thanks to both of you for the PR and the discussion.
I am fine with the changes. Just ping me when I should merge and you are also fine with it.

[philiptgrace]

This PR can be merged. Michael is working on a more comprehensive change on another branch, but this fix is good for now.