-
Notifications
You must be signed in to change notification settings - Fork 183
[9.0] feat: Add diracx secrets in SiteDirector #8239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[9.0] feat: Add diracx secrets in SiteDirector #8239
Conversation
Rebased on #8233 |
2675728
to
fc031a6
Compare
return cls.pilotAgentsDB.deletePilot(pilotIDs) | ||
|
||
# And list[str]???? | ||
# pilot_id>>>S<<< |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree this is not intuitive, and this was done before we introduced typing, but I think list[str]
is the "default" case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can't be list[str]
the default. In fact, if it is, it will instantly crash. In pseudo code:
if its_a_str:
# Add in db as if it was 1 stamp
if its_an_int:
# Transform to a list
# Add in db as if it was multiple IDs
If we have a list[str]
, the code will go in the last case and try to insert stamps as IDs
.
fc031a6
to
95fa831
Compare
e680870
to
3cd62e8
Compare
# AdditionalEnv[i] format: | ||
# {"secret": "1_l0v3_1c3cr34m", ...} | ||
# We merge them to have the right format: '("key" "value") (...)' | ||
currentEnv = " ".join([f"({key} {value})" for key, value in additionalEnv[i]]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See if we need also \"
pilotOptions.append(f"--pipInstallOptions={queueDict['PipInstallOptions']}") | ||
|
||
# FIXME: Get secret | ||
# if "secret" in queueDict: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be removed
# FIXME: Get secret | ||
# if "secret" in queueDict: | ||
# pilotOptions.append(f"--pilotSecret={queueDict['...']}") | ||
# FIXME: Get clientID |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If someone has an idea.
The more I think about it, the more I wonder whether it makes sense to go that far at the moment: may be we shouldn't try to adapt the DIRAC agents and should stop to the legacy adaptor within DIRAC. Why?
It's not super clear, but IIUC, in the roadmap we state that the A Any opinion? |
I tend to agree with you. |
Discussed above: won't be done. It will be done in DiracX directly. |
BEGINRELEASENOTES
*SiteDirector
CHANGE: Connect SiteDirector to DiracX to use secrets.
ENDRELEASENOTES