-
Notifications
You must be signed in to change notification settings - Fork 176
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
[v6r20] Mostly documentation #3675
Conversation
@@ -9,6 +9,7 @@ | |||
import errno | |||
import threading | |||
import sys | |||
from functools import reduce |
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.
This is a buggy import that keeps coming but I don't know why. probably someone has autopep8 configured for python3
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.
In this PR it was simply moved (it was few lines below), is it incorrect?
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.
Well, not incorrect per se, but rather useless now :-)
https://docs.python.org/2/library/functools.html#functools.reduce
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 see. I leave it here anyway.
Review, please |
@@ -136,7 +136,7 @@ def getSiteMask(self, printOutput=False, status='Active'): | |||
return result | |||
|
|||
############################################################################# | |||
def getBannedSites(self, gridType=[], printOutput=False): | |||
def getBannedSites(self, printOutput=False): |
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.
That's a change in the external API, if one wanted to be really strict, we should change major version number
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's anyway unused...
|
||
for seName in ses['Value']: | ||
# Ugly, ugly, ugly.. | ||
if ('-' not in seName) or ('_' in seName): |
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.
what is that for ?
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.
This snippet I copy-pasted from LHCbDIRAC, and I didn't write the original. I believe this was here for excluding the "SandboxSE" or "LogSE" and so on. But indeed this check doesn't make sense anymore, I will remove it.
The components don't need to be all resident on the same host, in fact it's common practice to have several hosts | ||
for large installations. | ||
|
||
As a general rul, services can be duplicated, |
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.
rul -> rule
Same can be said for executors: you can have many residing on different hosts. | ||
|
||
The same can't be said for agents. Some of them can be duplicated, BUT require a proper configuration, | ||
and for this you need to read further in the guide. |
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.
You may want to link the scalability page ?
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.
Which one?
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.
Starting Pilots3 via SiteDirectors | ||
================================== | ||
|
||
Since DIRAC v6r20, SiteDirectors can send "pilots2" or "pilots3". Pilots2 are the default, |
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.
you can use .. versionadded:: v6r20
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 turned off the whitespace changes, so I hope there is no bug there...
@@ -5,23 +5,29 @@ | |||
|
|||
""" | |||
|
|||
__RCSID__ = '$Id:$' |
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.
__RCSID__ = '$Id$'
|
||
ses = CSHelpers.getStorageElements() | ||
if not ses['OK']: | ||
gLogger.error(ses['Message']) |
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.
you need to return here, or something, otherwise the next line (ses['Value']
) gives an exception
The components don't need to be all resident on the same host, in fact it's common practice to have several hosts | ||
for large installations. | ||
|
||
As a general rul, services can be duplicated, |
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.
rule
|
||
As a general rul, services can be duplicated, | ||
meaning you can have the same service running on multiple hosts, thus reducing the load. | ||
There are only 2 cases of DIRAC services that have a "master/slave" concept, and these are the Configuration Service |
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.
Also the Matcher
cannot work more than once I believe.
""" | ||
DIRAC.Core.Utilities package | ||
""" | ||
__RCSID__ = "$Id$" | ||
from DIRAC.Core.Utilities.File import makeGuid, checkGuid, getSize, getGlobbedTotalSize, getGlobbedFiles, getCommonPath, getMD5ForFiles |
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.
This might give problems for extensions and should be noted in the transition guide, I think.
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.
clearly yes, I missed that one !
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.
Right, updated it. Thanks a lot.
* upstream/integration: Docs: autoformat MakeDoc.py Docs: dms doc correct the document Docs: add developper doc for SE and FC DOC: generate documentation for __init__ as well Doc: more DMS doc improve the documentation
DIRACGrid/WebAppDIRAC#291 is needed together with this PR |
return S_OK( reason ) | ||
return S_OK(reason) | ||
|
||
types_getSpaceTokenOccupancy = [(basestring, NoneType, list), (basestring, NoneType, list)] |
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 think you need to change this one to types_getFreeDiskSpace
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.
Thanks, fixed that and one more thing.
…ixes23 * 'v6r20-fixes23' of github.com:fstagni/DIRAC: Correct change certificate DNs correct typos add Accounting documentation correct the script options
BEGINRELEASENOTES
*docs
CHANGE: updated documentation on basic installation and pilots 3
ENDRELEASENOTES