Frontend config <attr type="expr" ...> issue
As per the frontend docs, type expr is a condor constant or condor expression
type: Type of the attribute. Supported types are 'int', 'string' and 'expr'. Type expr is equivalent to condor constant/expression in condor_vars.lst
But when used with following example, it looks like code tries to do an eval on this expression. This does not work since the value is condor expression.
<attr name="CLAIM_WORKLIFE" glidein_publish="True" job_publish="False" parameter="True" type="expr" value="ifThenElse(DynamicSlot =?= True, 600, -1)"/>
Above example results in frontend group crashing with following exception
[2013-12-06 09:25:25,453] INFO: Checking groups ['main'] [2013-12-06 09:25:25,454] WARNING: [<subprocess.Popen object at 0xd88d90>]: Traceback (most recent call last): File "/opt/glideinwms/frontend/glideinFrontendElement.py", line 1060, in <module> main(int(sys.argv), sys.argv, sys.argv) File "/opt/glideinwms/frontend/glideinFrontendElement.py", line 989, in main paramsDescript = glideinFrontendConfig.ParamsDescript(work_dir, group_name) File "/local/home/factory/master/glideinwms/frontend/glideinFrontendConfig.py", line 168, in __init__ self.expr_objs[k]=compile(val,"<string>","eval") File "<string>", line 1 ifThenElse(DynamicSlot =?= True, 600, -1) ^ SyntaxError: invalid syntax [2013-12-06 09:25:25,454] WARNING: Child main exited. Checking if it should be restarted.
So either this is a bug in the code or the docs are wrong and needs to be investigated.
#3 Updated by HyunWoo Kim over 4 years ago
- % Done changed from 0 to 70
I think I now understand how to resolve this ticket.
Here is what I did this morning:
I modified /usr/lib/python2.6/site-packages/glideinwms/frontend/glideinFrontendConfig.py
class ParamsDescript(JoinConfigFile): def __init__(self,base_dir,group_name): for k in self.data.keys(): type_str,val=self.data[k] if type_str=='EXPR': #HK5052 self.expr_objs[k]=compile(val,"<string>","eval") self.expr_objs[k]=val #HK5052
I modified /usr/sbin/glideinFrontendElement.py
glidein_params = copy.deepcopy(self.paramsDescript.const_data) for k in self.paramsDescript.expr_data.keys(): kexpr = self.paramsDescript.expr_objs[k] #HK5052 glidein_params[k] = glideinFrontendLib.evalParamExpr(kexpr, self.paramsDescript.const_data, glidein_el) glidein_params[k] = kexpr #HK5052
And then modified /etc/gwms-frontend/frontend.xml
<name="CLAIM_WORKLIFE" glidein_publish="True" job_publish="True" parameter="True" type="expr" value="ifThenElse(DynamicSlot =?= True, 600, -1)" />
[root@hepcloud-devfe hyunwoo]# service gwms-frontend reconfig
[hyunwoo@hepcloud-devfe ~]$ condor_submit condor-test.sub
So, two requirements have been fulfilled:
1. In the factory
[root@hepcloud-devfac hyunwoo]# condor_status -any -l -constraint 'MyType=="glideclient"' | grep CLAIM GlideinParamCLAIM_WORKLIFE = "ifThenElse(DynamicSlot =?= True, 600, -1)"
2. In the glidein
[root@ec2-52-88-150-230 glide_pBEdz2]# grep -r WORKLIFE * glidein_config : CLAIM_WORKLIFE ifThenElse(DynamicSlot =?= True, 600, -1) main/condor_vars.lst : GLIDEIN_CLAIM_WORKLIFE_DYNAMIC I 3600 + N N - main/condor_vars.lst : GLIDEIN_CLAIM_WORKLIFE I -1 + N N - main/condor_config.dedicated_starter.include : CLAIM_WORKLIFE = ifThenElse(DynamicSlot =?= True, $(GLIDEIN_CLAIM_WORKLIFE_DYNAMIC), $(GLIDEIN_CLAIM_WORKLIFE)) main/condor_config.multi_schedd.include : CLAIM_WORKLIFE = ifThenElse(DynamicSlot =?= True, $(GLIDEIN_CLAIM_WORKLIFE_DYNAMIC), $(GLIDEIN_CLAIM_WORKLIFE))
#4 Updated by HyunWoo Kim over 4 years ago
- Status changed from New to Feedback
- Assignee changed from HyunWoo Kim to Marco Mambelli
There are just 2 changes:
self.expr_objs[k]=val #HK to address ticket 5052 compile(val,"","eval")
glidein_params[k] = kexpr #HK to address ticket 5052. glideinFrontendLib.evalParamExpr(kexpr, self.paramsDescript.const_data, glidein_el)
I tested this changes and confirmed that glideclient shows a new attribute that I added in frontned.xml with type=eval
and also that this new attribute appears in the startd (see my previous comment above)
#5 Updated by Marco Mambelli over 4 years ago
- Status changed from Feedback to Assigned
- Assignee changed from Marco Mambelli to HyunWoo Kim
I have two feedbacks.
1. there is no need for comments like "#HK to address ticket 5052. glideinFrontendLib.evalParamExpr(kexpr, self.paramsDescript.const_data, glidein_el)”
Your name, old code and the ticket # (you can add it in the commit comment) can be retrieved form git and they add little information and make the code less readable. You should remove them.
2. The problem is bigger than the one solved:
In the configuration documentation (attar section description):
type is "Type of the attribute. Supported types are 'int', 'string' and 'expr'. Typeexpr is equivalent to condor constant/expression in condor_vars.lst”
But the type attribute is used in the code in 2 different ways:
- for parameters (e.g. /var/lib/gwms-frontend/vofrontend/params.cfg file) it defines CONST/EXPR
CONST (anything not “expr") means that the parameter is a constant, EXPR (“expr”)means that the parameter is a python expression evaluated each time it is used
- for condor variables it means that the variable for condor is a integer/I “int” or a quoted string/S “string” or a condor expression/C (unquoted string) “expr”
An attribute could be both a parameter and a condor variable, so the value “expr” has 2 conflicting meanings.
The solution proposed patches partially the problem but:
- it leaves the CONST/EXPR distinction, the multiple data structures to support it
- even if it treats both branches in a similar way removing the compilation
I would avoid that and go one of the following routes:
1. remove completely CONST/EXPR distinction, e.g. removing the paramsDescript.expr_data dictionary, ...
2. (my favorite) change the parsing of attr and leave the functionality in place:
2a. in creation/lib/cvWParamDict.py, function add_attr_unparsed_real
+ is_expr = False # attr_obj.type=="expr” is now used for HTCondor expression in condor variables
2b. add a try/except block around the compile instruction (in frontend/glideinFrontendConfig.py) to catch SyntaxError and say in the log file that there is an error in the attribute, without crashing
Solution 2 would allow in the future to add a new keyword. e.g. “pyexpr” to have again evaluated python expressions if needed.
- I would revert the changes (start a new clean branch v3/5052_2)
- I would do the changes for solution 2suggested above
I can di that if you prefer and send it back to you for feedback