Merge branch 'fix/update_config' into 'master'

Fix/update config

See merge request detectors/pycalibration!470

webservice/update_config.py

@@ -18,7 +18,6 @@ agipd_options = {
                        'msg': "Range list of maximum pulse indices "
                               "(--max-pulses start end step). "
                               "3 max input elements. "},
-        "calfile": {'typ': str},
     }
 
 data_mapping = {
@@ -32,7 +31,11 @@ available_options = {
     "MID_DET_AGIPD1M-1": [agipd_options, data_mapping]
 }
 
-formatter = lambda prog: argparse.HelpFormatter(prog, max_help_position=52)
+
+def formatter(prog):
+    return argparse.HelpFormatter(prog, max_help_position=52)
+
+
 parser = argparse.ArgumentParser(
     description='Request update of configuration', formatter_class=formatter)
 required_args = parser.add_argument_group('required arguments')
@@ -46,6 +49,13 @@ required_args.add_argument('--cycle', type=str, help='The facility cycle.')
 parser.add_argument('--apply', action='store_true',
                     help='Apply and push the requested '
                          'configuration update to the git.')
+parser.add_argument(
+    '--webservice-address',
+    type=str,
+    default="tcp://max-exfl016:5555",
+    help=('The port of the webservice to update '
+          'calibration configurations repository.')
+)
 # remove help calls as they will cause the argument parser to exit
 add_help = False
 if "-h" in sys.argv:
@@ -58,10 +68,11 @@ if "--help" in sys.argv:
 known, remaining = parser.parse_known_args()
 args = vars(known)
 karabo_id = args["karabo_id"]
-bool_keys = []
+webservice_address = args["webservice_address"]
 
+bool_keys = []
 
-# Avoid erros when karabo_id not yet given through the command line.
+# Avoid erros when karabo_id not given through the command line.
 if karabo_id is not None:
     # adding "no" bools to available options
     for det, val in available_options[karabo_id][0].items():
@@ -69,38 +80,46 @@ if karabo_id is not None:
             bool_keys.append(det)
 
     for b in bool_keys:
-        available_options[karabo_id][0]['no-{}'.format(det)] = {'typ': bool}
+        available_options[karabo_id][0][f'no-{b}'] = {'typ': bool}
 
     for exposed_options in available_options[karabo_id]:
         for option, info in exposed_options.items():
-            metavar = None
+            typ = info['typ']
+            choices = info.get('choices')
             if info['typ'] == list:
-                nargs = '+'
+                arguments = {
+                    "action": 'append',
+                    "nargs": '+',
+                }
                 typ = str
-                action = 'append'
                 # Avoid having a big line of choices in the help message.
                 if 'choices' in info.keys():
-                    metavar = option.upper()
+                    arguments.update({
+                        "metavar": option.upper(),
+                        "choices": choices,
+                    })
             else:
-                action = None
-                nargs = None
-                typ = info['typ']
+                arguments = {
+                    "choices": choices,
+                }
             # Add help messages
             help_msg = ""
             if 'msg' in info.keys():
                 help_msg += info['msg']
-            if 'choices' in info.keys():
-                choices = info['choices']
-            else:
-                choices = None
-            help_msg += f"Type: {info['typ'].__name__} ".upper()
-            parser.add_argument(f"--{option}", type=typ, action=action,
-                                metavar=metavar, nargs=nargs, choices=choices,
-                                help=help_msg)
-
-parser.add_argument('--instrument', type=str, choices=["CALLAB"],
-                    help='This is only used for testing purposes.')
 
+            help_msg += f"Type: {info['typ'].__name__} ".upper()
+            parser.add_argument(
+                f"--{option}",
+                type=typ,
+                help=help_msg,
+                **arguments
+            )
+
+parser.add_argument(
+    '--instrument',
+    type=str, choices=["CALLAB"],
+    help='This is only used for testing purposes.'
+)
 if add_help:
     sys.argv.append("--help")
 
@@ -135,10 +154,7 @@ for key, value in args.items():
                     new_conf[task][instrument][karabo_id][key.replace('no-', '')] = False  # noqa
                 # avoid saving the "no-"key into the updated config
                 continue
-            # Assure adding an empty string for new empty
-            # str. updates (e.g. calfile)
-            if isinstance(key, str) and (value == '' or value == ' '):
-                value = '""'
+
             # checking if data-mapping was updated.
             if key in data_mapping.keys():
                 if 'data-mapping' not in new_conf.keys():
@@ -160,7 +176,7 @@ print(f"Sending the following update: \n {pyaml}")
 print("-" * 80)
 con = zmq.Context()
 socket = con.socket(zmq.REQ)
-con = socket.connect("tcp://max-exfl016:5555")
+con = socket.connect(webservice_address)
 msg = "','".join(["update_conf", "SASEX", args["karabo_id"],
                   instrument, args["cycle"], args["proposal"],
                   json.dumps(new_conf), str(args["apply"])])

webservice/webservice.py

@@ -171,9 +171,8 @@ def merge(source: Dict, destination: Dict) -> Dict:
 
 
 def change_config(config, updated_config, karabo_id, instrument,
-                        cycle, proposal, apply=False) -> bytes:
-    """
-    Change the configuration of a proposal
+                  cycle, proposal, apply=False) -> bytes:
+    """ Change the configuration of a proposal
 
     If no proposal specific configuration yet exists, one is first created
     based on the default configuration of the proposal
@@ -182,44 +181,56 @@ def change_config(config, updated_config, karabo_id, instrument,
 
     :param config: repo config as given in YAML config file
     :param updated_config: a dictionary containing the updated config
-    :param instrument: the instrument to change config for
+    :param karabo_id: karabo detector identifier.
+    :param instrument: the instrument to change the config for
     :param cycle: the cycle to change config for
     :param proposal: the proposal to change config for
     :param apply: set to True to actually commit a change, otherwise a dry-run
                   is performed
     :return: The updated config to the requesting zmq socket
     """
-    # first check if a proposal specific config exists, if not create one
     repo = Repo(config['local-path'])
     repo.remote().pull()
-    prop_dir = os.path.join(repo.working_tree_dir, cycle)
-    os.makedirs(prop_dir, exist_ok=True)
-    fpath = "{}/{:06d}.yaml".format(prop_dir, int(proposal))
-    if not os.path.exists(fpath):
-        with open("{}/default.yaml".format(repo.working_tree_dir), "r") as f:
-            defconf = yaml.load(f.read(), Loader=yaml.FullLoader)
+    # A cycle directory is created, if doesn't exists.
+    cyc_dir = Path(repo.working_tree_dir, cycle)
+    cyc_dir.mkdir(exist_ok=True)
+    fpath = Path(cyc_dir, f"{int(proposal):06d}.yaml")
+
+    # In case cycle/proposal.yaml doesn't exist,
+    # new file is created based on default.yaml
+    if not fpath.exists():
+        with open(f"{repo.working_tree_dir}/default.yaml", "r") as f:
+            defconf = yaml.safe_load(f)
             subconf = {}
-            for action, instruments in defconf.items():
+            # Propsal.yaml should have dark, correct and data-mapping keys
+            # with all detector dictionaries of the changed instrument.
+            for action in ["dark", "correct"]:
                 subconf[action] = {}
-                if action != "data-mapping":
-                    subconf[action][instrument] = instruments[instrument]
-                else:
-                    subconf[action][karabo_id] = instruments[karabo_id]
+                subconf[action][instrument] = defconf[action][instrument]
+
+            # Copy data-mapping for all detectors of an instrument.
+            subconf["data-mapping"] = {}
+            for k_id in defconf["dark"][instrument].keys():
+                subconf["data-mapping"][k_id] = defconf["data-mapping"][k_id]
             with open(fpath, "w") as wf:
-                wf.write(yaml.dump(subconf, default_flow_style=False))
+                wf.write(yaml.safe_dump(subconf, default_flow_style=False))
+
     new_conf = None
     with open(fpath, "r") as rf:
-        existing_conf = yaml.load(rf.read(), Loader=yaml.FullLoader)
+        existing_conf = yaml.safe_load(rf)
         new_conf = merge(updated_config, existing_conf)
+
     if apply:
+        # Apply updated configuration to the proposal.yaml
+        # and push it to the calibration_configurations remote reporsitory.
         with open(fpath, "w") as wf:
-            wf.write(yaml.dump(new_conf, default_flow_style=False))
-        repo.index.add([fpath])
+            wf.write(yaml.safe_dump(new_conf, default_flow_style=False))
+        repo.index.add([str(fpath)])
         repo.index.commit(
-            "Update to proposal YAML: {}".format(datetime.now().isoformat()))
+            f"Update to proposal YAML: {datetime.now().isoformat()}")
         repo.remote().push()
     logging.info(Success.UPLOADED_CONFIG.format(cycle, proposal))
-    return yaml.dump(new_conf, default_flow_style=False).encode()
+    return yaml.safe_dump(new_conf, default_flow_style=False).encode()
 
 
 async def run_proc_async(cmd: List[str]) -> (int, bytes):