Design for importing hook config file in PTL

Developers
1

Hi All,

Please review the design document to support importing hoook config file in PTL
https://pbspro.atlassian.net/wiki/spaces/PD/pages/1223950391/Design+for+importing+hook+config+file+in+PTL

Please let me know if any comments or suggestions.

Thanks,
Shilpa Kodli

2

Thanks for writing this @shilpakodli. The design doc talks only about modify_hook_config(). But I guess there is no function to import the hook configuration file in first place. import_hook() accepts content_type as ‘application/x-python’ only. Shouldn’t we also add import_hook_config() ?

Also, I don’t quite understand “When “overwrite” attribute is set to True, then .CF file will be replaced by user requested file. If “overwrite” is set to False then only user requested values will be modified in existing config file.”. Could you please elaborate? What is the user requested file here ? There is no argument in modify_hook_config() that can take a user requested file .

How to we deal with mom hook config files under mom_priv dir ?

3

I also have same question for overwrite.
And please rename ‘id’ argument name with ‘hook_name’ as ‘id’ is build in method in python

4

@lsubramanian and @hirenvadalia Thanks for the inputs. When overwrite is set to ‘True’, entire .CF file will be replaced with user given attributes. User input should be in dict format.
I will change “file” with proper input format in EDD.

5

@shilpakodli Please add the link to forum in design doc. Please provide an example when overwrite is True and False.
What are the valid values for hook_type ? is it (site, PBS hook) or (site, pbshook), Values like hook for sitehook and pbshook for PBS hook falls inline with PBS implementation.
Please specify an example on how to do only import of an hook config.
Thanks,
Vishesh

6

Hi Shilpa,

As mentioned in EDD: If “overwrite” is set to False then only user requested values will be modified in existing config file". If a new attribute is to be added to hook config file then should it be used with overwrite as True or False?.

Thanks & Regards,
Ravi

7

@visheshh and @RKORavi, I will give an example about ‘overwrite’ here:

a = {‘attr1’: ‘value1’, ‘attr2’: ‘value2’}
self.modify_hook_config(test_hook, attrs=a)

When above method is called, it gets content from test_hook.CF file which will be under “/var/spool/pbs/server_priv/” and modifes values for ‘attr1’ and ‘attr2’.

If you want to add an additional parameter to config file then ‘overwrite’ will be set to False which is default value.

Let’s assume if user has added “overwrite=True” to method parameters then method call looks like below:
self.modify_hook_config(test_hook, attrs=a, overwrite=True)

In this case, entire config file will be overwritten with input dict ‘a’.

Let me know your inputs on this.

8

@shilpakodli . I think @RKORavi was asking if we want to add an value attr3:‘value3’ which is not present in the conf file
Consider {‘attr1’: ‘value1’, ‘attr2’: ‘value2’} is already present in the conf file.
should we use
a = {‘attr3’: ‘value3’}
self.modify_hook_config(test_hook, attrs=a)
OR
a = {‘attr1’: ‘value1’, ‘attr2’: ‘value2’,‘attr3’: ‘value3’}
self.modify_hook_config(test_hook, attrs=a, overwrite=True) ?

9

@visheshh You don’t have to give all the parameters in input dict if you are just adding a new attribute.
a = {‘attr3’: ‘value3’}
self.modify_hook_config(test_hook, attrs=a)

Above steps should add attr3 to existing config file.

And example which I have shown applies if attr1 and attr2 are already present in config file and if you want to change these attribute’s values.

10

Thanks @visheshh. Yes, that is exactly what i meant.

11

@shilpakodli Thank you for clearing this up for me. I have a followup question: what if a testcase is going to modify some of the config values & add a new attribute to config file. How will this be handled?
Then, do i need to call modify_hook_config twice? One for updating existing values & the other for adding a new attribute.

12

@RKORavi Not exactly. You can send bothe attributes in one call.

13

@RKORavi and @visheshh I have given one more example in EDD. Please have a look at it and let me know if you have any questions on it.

14

@shilpakodli Thanks for adding it in EDD, looks good to me.

15

@shilpakodli Looks good to me.

16

Hi,

Can you please mention the motivation for this change? why is Python’s json module not sufficient to modify a hook config file, which is basically a json?

Thanks,
Ravi

17

@agrawalravi90 If we use just Python’s json module then it will end up in 5-10 lines of code to just modify an attribute. Having an API for the same will reduce the code in test scripts and also easy for a person who has multiple tests to write which have this kind of steps.

18

Sounds good, please create a Motivation section and mention that this is a convenience method which will help test writers to get around writing those 5-10 lines of code every time they want to modify a hook config file.

19

One more thing, don’t we need a similar function in the MoM class as well? (for mom hooks?)

20

Since, hooks configuration files will be on server, we can have single function in server class.