-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
🐛 Source Facebook Marketing: Removed validation that blocked personal ad accounts during check
#32731
🐛 Source Facebook Marketing: Removed validation that blocked personal ad accounts during check
#32731
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Before Merging a Connector Pull RequestWow! What a great pull request you have here! 🎉 To merge this PR, ensure the following has been done/considered for each connector added or updated:
If the checklist is complete, but the CI check is failing,
|
check
) | ||
return False, message | ||
# Get Ad Account to check creds | ||
ad_account = api.account |
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.
isn't api.account a lazy attribute? if it is, let's add a small unit test for this change because if we'll somehow drop the log line, we can miss connectivity check at all
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.
Based on our tests and the documentation, the lazy property is indeed evaluated on first access. I've added a test to ensure that credentials validation is consistently checked.
The cached_property decorator only runs on lookups and only when an attribute of the same name doesn’t exist. When it does run, the cached_property writes to the attribute with the same name. Subsequent attribute reads and writes take precedence over the cached_property method and it works like a normal attribute.
… ad accounts during `check` (#32731)
… ad accounts during `check` (#32731)
What
Removed the validation that previously blocked personal ad accounts during the 'check' process.
How
Deleted the validation step for personal accounts and replaced it with a procedure to retrieve information about the ad account to validate credentials.
🚨 User Impact 🚨
This change allows users with personal accounts to use the Facebook Marketing connector.