python(feat): campaigns resource - #795
Brandon-Shippy wants to merge 2 commits into
Conversation
|
Python docs preview: https://sift-stack.github.io/sift/python/pr-795/ Deployed from |
3b1af5d to
41c3e0e
Compare
|
|
||
| report_id: str | ||
| report_name: str = "" | ||
| num_annotations: int = 0 |
There was a problem hiding this comment.
can we make this cleaner by moving this into a structure like:
rule_statistics: passed_count: .., open_count} etc
| return CampaignReportProto(report_id=kwargs["report_id"]) | ||
|
|
||
|
|
||
| def tag_names(tags: list[str] | list[Tag] | None) -> list[str]: |
There was a problem hiding this comment.
Seems like an odd place for this function
| ) | ||
|
|
||
| @property | ||
| def report_ids(self) -> list[str]: |
There was a problem hiding this comment.
I don't think we need this - a user can do the list comprehension themselves if they really need this.
| return [r.report_id for r in self.reports] | ||
|
|
||
| @property | ||
| def resolved_reports(self) -> list[Report]: |
There was a problem hiding this comment.
Should this just be reports and the existing reports to report_summaries
| from sift_client.sift_types.run import Run | ||
|
|
||
|
|
||
| class CampaignReport(BaseModel): |
There was a problem hiding this comment.
CampaignReportSummary would be more descriptive
| name: str | ||
| client_key: str | None = None | ||
| organization_id: str | None = None | ||
| create_default_report: bool | None = None |
There was a problem hiding this comment.
Do we really want to support this? Feels like a bodge and not first class. Would prefer a different method.
| return [tag.name if isinstance(tag, Tag) else tag for tag in tags or []] | ||
|
|
||
|
|
||
| class Campaign(BaseType[CampaignProto, "Campaign"]): |
There was a problem hiding this comment.
Some helpers around "add_reports`, etc. would be nice.
| *, | ||
| reports: list[Report] | list[str] | None = None, | ||
| runs: list[Run] | list[str] | None = None, | ||
| from_campaign: str | Campaign | None = None, |
There was a problem hiding this comment.
can this just be "campaign"?
| ) | ||
| return self._apply_client_to_instance(created) | ||
|
|
||
| async def get_or_create(self, create: CampaignCreate | dict) -> Campaign: |
There was a problem hiding this comment.
This is generally something we should avoid. Any specific reason we have this for campaigns?
| updated = await self._low_level_client.update_campaign(update) | ||
| return self._apply_client_to_instance(updated) | ||
|
|
||
| async def add_reports( |
There was a problem hiding this comment.
add_reports_to_campaign... etc
Description
Adds a campaigns resource to
sift_client. A campaign groups the reports from a series of runs, so a whole test program can be reviewed at once.CampaignsAPIAsyncwith the usualget,list_,find,create,update,archive,unarchive, plusget_or_create. Wired up asclient.campaignsCampaignsLowLevelClientwrapsCampaignService; the high-level client has zero proto importssift_types/campaign.pyadd_runs(...)andadd_reports(...)grow a campaign without replacing its report listreport_summaries(...)returns the per-report rule rollup;Campaign.report_summaries()andCampaign.runsread it back from the campaignRunCreate.create_default_reportso a run generates the report a campaign holdsValidation