update obsdb columns or add entries from obsfiledb #794
Conversation
There was a problem hiding this comment.
Thanks. I'm a bit confused about why the thing called "update_columns" seems so intrinsically tied to obsfiledb. Is this mostly because one needs a way to get "all the obs" rescanned?
It seems that the basic behavior here is that when you "update-obsdb" you
- generate a list of books to operate on
- update the obsdb for each book, which includes (a) creating the entry if it doesn't exist and (b) updating the entry to the latest schema if it does exist.
If that's right then I think you should make that more clear in the docs, and also rename "update_columns" to reflect what it does more directly, which is "rescan-from-obsfiledb" or something like that.
Is there a way to "update" or "rerun" or "add" just a single obs, provided by path? I think that's an important capability to have. For testing of the "update columns" aspect of this.
| @@ -125,16 +131,28 @@ def main(config: str, | |||
| accept_type = [booktype] | |||
|
|
|||
| config_dict = yaml.safe_load(open(config, "r")) | |||
| obsfiledb_based_update = ("obsfiledb" in config_dict) and update_cols | |||
There was a problem hiding this comment.
Why bother testing obsfiledb in config_dict here? It is more important to fail with information, later, when you actually try to use the obsfiledb (because it could be invalid for a number of reasons -- it is None, the file doesn't exist, etc.).
| logger.error("Trying to update based on obsfiledb, but none specified in the configuration file") | ||
|
|
There was a problem hiding this comment.
So is this a fatal error or not?
| if last_mod < tback:#Ignore older directories | ||
|
|
||
| #Look through obsfiledb obs_ids | ||
| if obsfiledb_based_update: |
There was a problem hiding this comment.
I request that you convert the content in the if and else blocks to functions.
| @@ -298,7 +338,9 @@ def get_parser(parser=None): | |||
| parser.add_argument("--booktype", default="both", type=str, | |||
| help="Select book type to look for: obs, oper, both(default)") | |||
| parser.add_argument("--overwrite", action="store_true", | |||
| help="If true, writes over existing entries") | |||
| help="If True, overwrite existing entries") | |||
| parser.add_argument("--update_cols", action="store_true", | |||
There was a problem hiding this comment.
We use form "--update-cols" instead of "--update_cols", in site_pipeline.
|
It looks for the locations of the books to be rescanned in the obsfiledb. |
Monolithic as per Matthew's request. see closed PR#677 for how the script worked when separate. Now it's just a matter of using an --update_cols argument to load books from the ones listed in the obsfiledb and fill out columns or even whole entries in the obsdb.