You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "kalai-logicsoft (via GitHub)" <gi...@apache.org> on 2024/08/29 09:19:47 UTC
[PR] improve documentation(docs): clarify URL encoding requirement for connection strings [superset]
kalai-logicsoft opened a new pull request, #30047:
URL: https://github.com/apache/superset/pull/30047
### SUMMARY
Enhance the documentation of the connection string used in the backend of Superset by adding additional information for improved understanding.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
|State|Image|
|-------|------|
|Before||
|After||
### TESTING INSTRUCTIONS
1. Start the documentation site following these [instructions](https://superset.apache.org/docs/contributing/howtos#local-development).
2. Navigate to the `SQL Server` section on `Connecting to Databases` page.
3. Verify that the additional notes information is present and accurate.
### ADDITIONAL INFORMATION
- [x] Has associated issue:
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
See more discussions on this issue #30045
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs: improve documentation(docs): clarify URL encoding requirement for connection strings [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #30047:
URL: https://github.com/apache/superset/pull/30047#discussion_r2054905894
##########
docs/docs/configuration/databases.mdx:
##########
@@ -1287,6 +1287,13 @@ The connection string for SQL Server looks like this:
mssql+pyodbc:///?odbc_connect=Driver%3D%7BODBC+Driver+17+for+SQL+Server%7D%3BServer%3Dtcp%3A%3Cmy_server%3E%2C1433%3BDatabase%3Dmy_database%3BUid%3Dmy_user_name%3BPwd%3Dmy_password%3BEncrypt%3Dyes%3BConnection+Timeout%3D30
```
+:::note
+You'all might have noticed that some special form of charecters are used in above connection string. For example see the `odbc_connect` parameter, the value is `Driver%3D%7BODBC+Driver+17+for+SQL+Server%7D%3B` which is URL encoded form of `Driver={ODBC+Driver+17+for+SQL+Server};`. It's important to give the connection string as URL encoded format.
+
+For more information about this check the [sqlalchemy documentation](https://docs.sqlalchemy.org/en/20/core/engines.html#escaping-special-characters-such-as-signs-in-passwords). Which says `When constructing a fully formed URL string to pass to create_engine(), special characters such as those that may be used in the user and password need to be URL encoded to be parsed correctly.. This includes the @ sign.`
+:::
+
+
Review Comment:
```suggestion
:::note
You might have noticed that some special charecters are used in the above connection string. For example see the `odbc_connect` parameter. The value is `Driver%3D%7BODBC+Driver+17+for+SQL+Server%7D%3B` which is a URL-encoded form of `Driver={ODBC+Driver+17+for+SQL+Server};`. It's important to give the connection string is URL encoded.
For more information about this check the [sqlalchemy documentation](https://docs.sqlalchemy.org/en/20/core/engines.html#escaping-special-characters-such-as-signs-in-passwords). Which says `When constructing a fully formed URL string to pass to create_engine(), special characters such as those that may be used in the user and password need to be URL encoded to be parsed correctly. This includes the @ sign.`
:::
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs: improve documentation(docs): clarify URL encoding requirement for connection strings [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #30047:
URL: https://github.com/apache/superset/pull/30047#issuecomment-2326947796
Thanks for the pull request! You wrote:
> See more discussions on this issue https://github.com/apache/superset/issues/30045
But that issue looks unrelated. Could you have meant to link a separate issue?
Is this addition specific to SQL Server only or should it be added to this page in general?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs: improve documentation(docs): clarify URL encoding requirement for connection strings [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #30047:
URL: https://github.com/apache/superset/pull/30047
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs: improve documentation(docs): clarify URL encoding requirement for connection strings [superset]
Posted by "kalai-logicsoft (via GitHub)" <gi...@apache.org>.
kalai-logicsoft commented on PR #30047:
URL: https://github.com/apache/superset/pull/30047#issuecomment-2327945513
> Thanks for the pull request! You wrote:
>
> > See more discussions on this issue #30045
> > But that issue looks unrelated. Could you have meant to link a separate issue?
>
> Is this addition specific to SQL Server only or should it be added to this page in general?
Thanks for your appreciation,
From the `SQLAlchemy` documentation I understood that it applies to all database connection strings which uses `SQLAlchemy` to connect. So, it will apply to page level.
And one more information related to this is already the documentation for [AWS Athena](https://superset.apache.org/docs/configuration/databases/#aws-athena) has information related to this but specific to Athena database.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs: improve documentation(docs): clarify URL encoding requirement for connection strings [superset]
Posted by "kalai-logicsoft (via GitHub)" <gi...@apache.org>.
kalai-logicsoft commented on PR #30047:
URL: https://github.com/apache/superset/pull/30047#issuecomment-2328580502
> > But that issue looks unrelated. Could you have meant to link a separate issue?
I had provided the wrong link earlier. I’ve now updated it with the correct issue discussion. I apologize for any confusion caused.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org