-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PROD-2709 Fix Email Connector logs so they use configuration key instead of name #5286
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
fides Run #10063
Run Properties:
|
Project |
fides
|
Branch Review |
refs/pull/5286/merge
|
Run status |
Passed #10063
|
Run duration | 00m 39s |
Commit |
2361c26677 ℹ️: Merge 6d119ccfd64baaddc0ceb3f7190d314217e8a63d into d5986a5c39469165ef8743dd9f0a...
|
Committer | erosselli |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
4
|
Upgrade your plan to view test results. | |
View all changes introduced in this branch ↗︎ |
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.
thanks for jumping on this, I don't know how long this has been here, but this will really help troubleshooting in places.
You might look into adding some sort of property on the conneciton config that returns name or key and use that everywhere
"dataset_name": self.configuration.name | ||
or self.configuration.key, | ||
"privacy_request_id": privacy_request.id, | ||
"collection_name": self.configuration.name, | ||
"collection_name": self.configuration.name | ||
or self.configuration.key, |
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.
a few more places where you can use your new property!
"connection_key": self.configuration.key, | ||
"dataset_name": self.configuration.name, | ||
"collection_name": self.configuration.name, | ||
"dataset_name": self.configuration.name | ||
or self.configuration.key, | ||
"collection_name": self.configuration.name | ||
or self.configuration.key, |
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.
same here -
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.
🎉
7bc68b0
to
08fe2e5
Compare
fides Run #10068
Run Properties:
|
Project |
fides
|
Branch Review |
main
|
Run status |
Passed #10068
|
Run duration | 00m 39s |
Commit |
0a0ea4f400: PROD-2709 Fix Email Connector logs so they use configuration key instead of name...
|
Committer | erosselli |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
4
|
Upgrade your plan to view test results. | |
View all changes introduced in this branch ↗︎ |
Closes PROD-2709
Description Of Changes
Adds the
name_or_key
property toConnectionConfig
, and updates all email connectors to do two things:ExecutionLog
instances created will use thename_or_key
property instead of just the nameThis is because the
name
is an optional property for the connection config.Steps to Confirm
Pre-Merge Checklist
CHANGELOG.md
main
downgrade()
migration is correct and works