-
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
Removing the connector template properties #4128
Conversation
…ting them during creation
Passing run #4327 ↗︎
Details:
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. |
config = SaaSConfig(**load_config_from_string(template.config)) | ||
authentication = config.client_config.authentication | ||
authorization_required = ( | ||
authentication.strategy |
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 was a bit hard for me to read because it's doing a nested comparison instead of using and
, I think something like this might be a bit more readable:
In [1]: foo = None
In [2]: bar = True if foo and foo.test else False
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.
Good call!
@@ -71,6 +74,14 @@ def _load_connector_templates(self) -> None: | |||
config_dict = load_config(config_file) | |||
connector_type = config_dict["type"] | |||
human_readable = config_dict["name"] | |||
user_guide = config_dict.get("user_guide") | |||
authentication = config_dict["client_config"].get("authentication") | |||
authorization_required = ( |
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 note as below
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4128 +/- ##
==========================================
+ Coverage 87.50% 87.51% +0.01%
==========================================
Files 326 326
Lines 20324 20323 -1
Branches 2640 2640
==========================================
+ Hits 17784 17786 +2
+ Misses 2081 2078 -3
Partials 459 459
☔ View full report in Codecov by Sentry. |
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.
LGTM, not sure if external/unsafe checks need to be run though
Closes #4123
Description Of Changes
Removing the
authorization_required
anduser_guide
properties fromConnectorTemplate
. Calculating these values on the fly required the parsing of the SaaS config YAML and caused other threads to block.Code Changes
Steps to Confirm
Salesforce Classic
connector in the System Integration tab, this connector hasauthorization_required
anduser_guide
, The page should show anAuthorize integration
button and have a link to external documentation.Pre-Merge Checklist
CHANGELOG.md