python(feat): Report templates service #145
Conversation
| archived_date=archived_date, | ||
| ) | ||
|
|
||
| field_mask = FieldMask(paths=["name", "description", "tags", "rules", "archived_date"]) |
There was a problem hiding this comment.
I was originally thinking we could be smarter about this and check each field to see which ones have actually changed, and just include those in the fieldmask, but I'm not sure if that buys us anything
There was a problem hiding this comment.
Since this is "private" that should be fine? It's only accessible via the create_or_update_* method so I can't see any potential footguns but I'd just double check and consider this a bit more.
There was a problem hiding this comment.
I decided to leave this as-is for now.. I can't yet think of a way this might cause issues and this way allows us to keep the logic simpler for now. Let me know if you've had other thoughts in the meantime though
| archived_date=archived_date, | ||
| ) | ||
|
|
||
| field_mask = FieldMask(paths=["name", "description", "tags", "rules", "archived_date"]) |
There was a problem hiding this comment.
Since this is "private" that should be fine? It's only accessible via the create_or_update_* method so I can't see any potential footguns but I'd just double check and consider this a bit more.
| from sift_py.report_templates.config import ReportTemplateConfig | ||
|
|
||
|
|
||
| class ReportTemplateService: |
There was a problem hiding this comment.
Could we go pretty aggressive with the doc comments for this class? OPtional for the private method unless you think it'll help other devs
There was a problem hiding this comment.
Updated! For the private ones I just added a docstring to the _update method to try to clarify how I'm doing things there.
| @@ -31,6 +32,25 @@ def read_and_validate(path: Path) -> TelemetryConfigYamlSpec: | |||
| return _validate_yaml(raw_config) | |||
|
|
|||
|
|
|||
| def load_report_templates(paths: List[Path]) -> List[ReportTemplateConfig]: | |||
There was a problem hiding this comment.
I think we should start breaking this yaml module apart. It is starting to feel awkward to have this be the dumping ground for parsing different yaml specs.
I think what make sense to me is having a top-level module along-side report_template maybe called yaml_specs and having the specs and parsing logic be separated by domain i.e. yaml_specs/report_template, yaml_specs/rule, yaml_spec/telemetry_config or something like that. We don't have to do this for everything right now, but can we move report templates logic out of this module? We can do the same for rule for your rules PR. We'll move telemetry config stuff out later.
There was a problem hiding this comment.
Sure, I think that sounds good! I'll go ahead and create this for report templates and then we can update this for the other domains the next time we touch them.
There was a problem hiding this comment.
Updated, let me know how that looks!
This PR adds a
ReportTemplateServicethat allows users to create, update, fetch, and archive report templates. There are also examples of how to define report templates in python, in yaml, and how to fetch, update, and archive those created templates. This also notably adds theReportTemplateConfigas a pydantic model.