refactor: more structured feed() loop and consistent logs #27
Conversation
| json.dump([asdict(o) for o in orgs], f, indent=2, ensure_ascii=False) | ||
|
|
||
|
|
||
| def get_target_universe( |
There was a problem hiding this comment.
| def get_target_universe( | |
| def get_target_universe_objects( |
Needs to be more descriptive, not sure about my suggestion.
| t_count[element_class] += len(object_ids) | ||
| active_orgs[element_class].append(org) | ||
| new_object_ids += object_ids | ||
| verbose_print(f"Fetching target {element_class.value}...") |
There was a problem hiding this comment.
I've been confused by the "target" naming. Makes sense after scratching my head but somehow, target could be the universe topic too (the one we're writing to)...
There was a problem hiding this comment.
True... I didn't like "new" for about the same reason (could be we're creating a new topic).
How about "intended", "upcoming", or "goal"? Any better idea?
There was a problem hiding this comment.
There's "expected" too, but I', worried about potential interference with tests (so avoiding actual/expected).
There was a problem hiding this comment.
I kind of like upcoming, goes well with the flow.
| f"Found {len(target_object_ids)} {element_class.value} matching the target universe." | ||
| ) | ||
|
|
||
| # TODO: don't run if reset==True? |
There was a problem hiding this comment.
Indeed, but pretty cheap, doesn't matter IMO.
There was a problem hiding this comment.
Can even be a feature: the log will confirm the topic is indeed empty.
There was a problem hiding this comment.
Doesn't matter for perf for sure, but I was more thinking about code intent/readability.
You're right though that confirmation is a good idea, if only because we could have something added back to the topic between reset and here.
There was a problem hiding this comment.
Re readability, it probably reads better without another if.
| verbose_print("Computing topic updates...") | ||
| additions = sorted(set(target_object_ids) - set(existing_object_ids)) | ||
| removals = sorted(set(existing_object_ids) - set(target_object_ids)) | ||
| if n := len(removals) > REMOVALS_THRESHOLD: |
There was a problem hiding this comment.
| if n := len(removals) > REMOVALS_THRESHOLD: | |
| if (n := len(removals)) > REMOVALS_THRESHOLD: |
| print(f"- Adding {len(additions)} {element_class.value}...") | ||
| datagouv.put_topic_elements(conf.topic, element_class, additions, ADDITIONS_BATCH_SIZE) | ||
|
|
||
| # TODO: don't run if reset==True? |
There was a problem hiding this comment.
Same here, cheap and log will be consistent.
| # FIXME: remove when front uses the new file path | ||
| # retrocompatibility | ||
| copyfile( | ||
| f"dist/organizations-datasets-{conf.env}.json", f"dist/organizations-{conf.env}.json" |
Prep work opendatateam/udata-front-kit#961.
Example log:
In verbose: