Fix/manual submit confirmation

Description

Answering n to the confirmation prompt for submitting jobs via the manual launch script would launch the jobs anyway.

As Thomas pointed out, this is because Prompt.ask returns the response (in this case an "n"), which the if branch did not check properly.

Changed the check to use Confirm.ask instead, which returns a bool True/False based on the response.

Did not notice this during testing since the prompt had False as the default value, whenever I didn't want to submit the jobs I just pressed enter instead of writing in n.

While testing I also noticed that MyMdC has different behaviour for the xcal account and 'normal' accounts: xcal does not pass a scope through during auth, normal accounts require one. Added a simple check to only include the scope the account is not a dummy one (ending with @example.com).

How Has This Been Tested?

Manually

Relevant Documents (optional)

Types of changes

• Bug fix (non-breaking change which fixes an issue)

Checklist:

• My code follows the code style of this project.

Reviewers

@kluyvert @schmidtp

added Waiting for review label

assigned to @roscar

Does it work pass scope='public' all the time?

Otherwise LGTM.

changed milestone to %3.11.2

changed milestone to %3.11.3

pressing enter to reply is not my skill.

You mean on this section?

        # With a real user the scope must be set to public
        if not str(config["metadata-client"]["user-email"]).endswith("example.com"):
            data["scope"] = "public"

The docs say that the only option is scope=public, not setting the scope or having it be something other than public gives a response of {"error":"invalid_scope","error_description":"The requested scope is invalid, unknown, or malformed."}. So you'd think setting scope=public would work all the time, but the cal tokens seem to be an exception to this and does the opposite: it only works if scope isn't set or is empty, setting it to a value gives the invalid scope error.

So apart from the cal token it's required all the time.

I guess this could be added to the configurations so that it can be set in the files/with env vars, or added as a cli argument, but since the categories seem to be "is xcal" and "is not xcal" it didn't seem worth doing.

OK, fair enough. Might be worth checking with @maial if this is a bug of some kind, I guess.

Hey,

I think that if you have doubts, then the information we provided on the page is not enough.

The idea is that if you use "Authorization Code flow" you need to set the scope as "public".

For the standard "client Credentials flow" (the one we use) you should leave it empty because in myMdC we never set the dafault_scopes in Doorkeeper (details: https://doorkeeper.gitbook.io/guides/configuration/scopes)

I have been trying to find some time for a long time to check if adding "default_scopes :public" to myMdC would break all existing connections... Is your key in the test_metadata? If yes we can even test it quickly.

Thank you.

Thanks Luis. I think we always use the 'client credentials' flow - this is where the client has a secret and gets a token with no user interaction, right?

What's confused us is that it that (from what Robert said), the token request seems to need 'public' scope unless we use the existing client ID & secret set up for calibration, in which case it needs to be empty.

This isn't really important, though - it's just a helper script, and we have a workaround. It just seems odd that different tokens need different scope settings to do the same thing.

merged

mentioned in commit a9131550

removed Waiting for review label