Skip to content

RG config client — CRUD, models and versioned generate methods#38

Open
carlosfunk wants to merge 2 commits into
mainfrom
dm-3731-rg-config
Open

RG config client — CRUD, models and versioned generate methods#38
carlosfunk wants to merge 2 commits into
mainfrom
dm-3731-rg-config

Conversation

@carlosfunk

@carlosfunk carlosfunk commented Jul 19, 2026

Copy link
Copy Markdown
Member

Adds client support for DataMasque ruleset-generation configs: server-side masks-per-label configs consumed by the versioned generate APIs.

Commits

  • feat(client): add RG config CRUD and versioned generate methods

What's in it

  • RGConfig pydantic model + RGConfigClient mixin: list/get/create/update/delete + get_default_rg_config_yaml() against rg/configs/
  • Versioned generate methods mirroring the discovery *_from_config split: generate_ruleset_with_rg_config (v3), generate_file_ruleset_with_rg_config (v2), start_async_ruleset_generation_with_rg_config (v2), start_async_ruleset_generation_from_csv_with_rg_config (v2). Old methods keep the old endpoints (server default RG config).
  • Old generate request models reject rg_config and point callers at the new methods.
  • CSV/multipart note: rg_config=None is sent as an empty form field; DRF coerces '' to null on relational fields (verified end-to-end against a live server).

@carlosfunk
carlosfunk marked this pull request as draft July 19, 2026 23:01
@carlosfunk carlosfunk changed the title DM-3731: RG config client — CRUD, models and versioned generate methods (DM-3958) RG config client — CRUD, models and versioned generate methods Jul 20, 2026
@carlosfunk carlosfunk self-assigned this Jul 20, 2026
- add RGConfig pydantic model and RGConfigClient mixin: list/get/create/update/delete + defaults yaml
- add generate_ruleset_with_rg_config (v3) and file/async/csv variants (v2), mirroring the discovery from_config split
- old generate request models reject rg_config and point callers at the new methods
- document dependent-RGC behaviour on ruleset library delete/usage APIs
@carlosfunk
carlosfunk marked this pull request as ready for review July 27, 2026 03:00

@cph-datamasque cph-datamasque left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly minor issues.

Do we want a RGConfigNotFoundError exception type to mirror the handling of 400 responses for other APIs?

Another thing that needs to be done at some point: the docs are out of date and need regenerating. Up to you if you want to do that in this MR, or we could raise a ticket to do it another day.

Comment thread datamasque/client/models/discovery.py Outdated
def _reject_rg_config(cls, data: Any) -> Any:
if isinstance(data, dict) and "rg_config" in data:
raise ValueError(
"`rg_config` is not accepted by the v2 ruleset-generation request; "

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In these error messages, don't talk about different versions of the API, as that's meant to be transparent to users of this library.

Comment thread datamasque/client/models/discovery.py Outdated
def _reject_rg_config(cls, data: Any) -> Any:
if isinstance(data, dict) and "rg_config" in data:
raise ValueError(
"`rg_config` is not accepted by the unversioned file-ruleset-generation request; "

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above


@field_validator("connection", mode="before")
@classmethod
def _unwrap_connection(cls, value: Any) -> Any:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a separate thought: can we do anything with this meh Any typing on these helpers, and their duplication? Maybe a generic method that is like unwrap[T1, T2](value: Union[T1, T2]) -> Optional[T2] where T1 is the object type and T2 is the id value type, T1 being typedef'd to be a protocol with id field?

Comment thread datamasque/client/discovery.py Outdated

Generation runs asynchronously on the server.
Poll `get_async_ruleset_generation_task_status` until it returns
`AsyncRulesetGenerationTaskStatus.finished`,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(minor) Does this fit on the previous line to make it easier to read? (Line length can be 120.) Same below at 111.

Comment thread datamasque/client/discovery.py Outdated
"""
Generate ruleset(s) from a schema discovery CSV report with a selected RG config.

Like `start_async_ruleset_generation_from_csv`, but posts to the v2 endpoint with a required

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semantic breaking, as per CONTRIBUTING.rst.

Comment thread datamasque/client/discovery.py Outdated
(`None` for the server's default RG config).
"""

data = generation_request.model_dump(exclude_none=True, mode="json")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern is duplicated - worth a shared helper?

(i.e., it must have been previously created or retrieved from the server).
"""

if config.id is None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also need to check config_yaml here - see the DCL API

Comment thread datamasque/client/rg_configs.py Outdated
No-op if no such config exists.
"""

matching = [config for config in self.list_rg_configs() if config.name == name]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do a fetch of just the one by name (_get_rg_config_id_by_name) rather than enumerating all of them?

Comment thread datamasque/client/models/rg_config.py Outdated
"""
Represents a named, persisted YAML ruleset-generation (RG) configuration.

An RG config maps discovered labels to masks; ruleset generation applies it to a

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sembr here

* Add `RGConfigNotFoundError`, raised by all four `*_with_rg_config` methods when the server rejects the config id.
* Guard `update_rg_config` against a YAML-less config, matching the discovery config library client.
* Resolve `delete_rg_config_by_name_if_exists` with the `name_exact` filter instead of enumerating every config.
* Fold the duplicated `rg_config` request-body handling into `_post_with_rg_config`.
* Drop API version numbers from the `rg_config`-rejection error messages and the changelog.
* Apply semantic line breaks to the RG config docstrings and add the missing docs stubs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants