-
Notifications
You must be signed in to change notification settings - Fork 16
Conversation
Misc cleanup associated with change
…xpires_in value in the access token response
connector_param: requester_email | ||
- name: email | ||
identity: email | ||
body: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In YAML the pipe |
is used to indicate a "block scalar" value (multiline text). This allows us to denote JSON payloads inline with YAML while avoiding having to escape single or double quotes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's cool, good thing i got to review this PR and learn something new :)
@@ -169,6 +169,7 @@ class ConnectorParam(BaseModel): | |||
"""Used to define the required parameters for the connector (user-provided and constants)""" | |||
|
|||
name: str | |||
description: Optional[str] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding this to include descriptions for fields that need more of an explanation than what can just be assumed from the name alone. Currently there is no additional logic in Fidesops that makes use of this information but this could be helpful when for when we have a UI to create/manage SaaS configs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generally looks good, one very minor comment on whether to include a certain config in the .toml
but if i'm mistaken on that then let me know and i'll 👍
saas_config.toml
Outdated
requester_email = "" | ||
identity_email = "" | ||
erasure_identity_email = "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this is dynamically generated in the fixture (which i like), does it need to be in the toml
? (specifically the erasure_identity_email
). it seems like it could be misleading. but my understanding may be off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point, removed!
Removing unnecessary config value from saas_config.toml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this all looks good so i'm going to approve but @galvana seems like a merge conflict arose (just FYI)
Purpose
Adds erasure functionality for Outreach.
Changes
The following endpoints were added as
deletes
prospects
POST /api/v2/complianceRequests
recipients
POST /api/v2/complianceRequests
Checklist
CHANGELOG.md
fileCHANGELOG.md
file is being appended toUnreleased
section in an appropriate category. Add a new category from the list at the top of the file if the needed one isn't already there.Ticket
Fixes #618