sfirke commented on code in PR #24751:
URL: https://github.com/apache/superset/pull/24751#discussion_r1271543829


##########
docs/docs/databases/redshift.mdx:
##########
@@ -18,8 +20,47 @@ You'll need to the following setting values to form the 
connection string:
 - **Database Name**: Database Name
 - **Port**: default 5439
 
-Here's what the connection string looks like:
+
+### psycopg2
+
+Here's what the SQLALCHEMY URI looks like:
 
 ```
 redshift+psycopg2://<userName>:<DBPassword>@<AWS End Point>:5439/<Database 
Name>
 ```
+
+
+### redshift_connector
+
+Here's what the SQLALCHEMY URI looks like:
+
+```
+redshift+redshift_connector://<userName>:<DBPassword>@<AWS End 
Point>:5439/<Database Name>
+```
+
+
+#### Using IAM-based credentials with Redshift cluster:
+
+[Amazon redshift 
cluster](https://docs.aws.amazon.com/redshift/latest/mgmt/working-with-clusters.html)
 also supports to generate temporary IAM-based database user credentials.

Review Comment:
   "Supports to generate" is unclear.  
   
   "also supports generating temporary", if it is doing the generating?  I 
can't confidently suggest a replacement since I'm unfamiliar with this process.



##########
docs/docs/databases/athena.mdx:
##########
@@ -33,10 +33,7 @@ following connection string:
 
awsathena+rest://{aws_access_key_id}:{aws_secret_access_key}@athena.{region_name}.amazonaws.com/{schema_name}?s3_staging_dir={s3_staging_dir}&...

Review Comment:
   It won't let me mark it on line 29 but that should be "you can also use 
**the** PyAthena library" -- maybe that can be fixed in this PR even though 
it's not an addition



##########
docs/docs/databases/redshift.mdx:
##########
@@ -18,8 +20,47 @@ You'll need to the following setting values to form the 
connection string:
 - **Database Name**: Database Name
 - **Port**: default 5439
 
-Here's what the connection string looks like:
+
+### psycopg2
+
+Here's what the SQLALCHEMY URI looks like:
 
 ```
 redshift+psycopg2://<userName>:<DBPassword>@<AWS End Point>:5439/<Database 
Name>
 ```
+
+
+### redshift_connector
+
+Here's what the SQLALCHEMY URI looks like:
+
+```
+redshift+redshift_connector://<userName>:<DBPassword>@<AWS End 
Point>:5439/<Database Name>
+```
+
+
+#### Using IAM-based credentials with Redshift cluster:
+
+[Amazon redshift 
cluster](https://docs.aws.amazon.com/redshift/latest/mgmt/working-with-clusters.html)
 also supports to generate temporary IAM-based database user credentials.
+
+Your superset app's [IAM role should have 
permissions](https://docs.aws.amazon.com/redshift/latest/mgmt/generating-iam-credentials-role-permissions.html)
 to call the `redshift:GetClusterCredentials` operation.
+
+You have to define following arguments in Superset's redshift database 
connection UI under ADVANCED --> Others --> ENGINE PARAMETERS.
+

Review Comment:
   Missing word: define **the** following



##########
docs/docs/databases/redshift.mdx:
##########
@@ -18,8 +20,47 @@ You'll need to the following setting values to form the 
connection string:
 - **Database Name**: Database Name
 - **Port**: default 5439
 
-Here's what the connection string looks like:
+
+### psycopg2
+
+Here's what the SQLALCHEMY URI looks like:
 
 ```
 redshift+psycopg2://<userName>:<DBPassword>@<AWS End Point>:5439/<Database 
Name>
 ```
+
+
+### redshift_connector
+
+Here's what the SQLALCHEMY URI looks like:
+
+```
+redshift+redshift_connector://<userName>:<DBPassword>@<AWS End 
Point>:5439/<Database Name>
+```
+
+
+#### Using IAM-based credentials with Redshift cluster:
+
+[Amazon redshift 
cluster](https://docs.aws.amazon.com/redshift/latest/mgmt/working-with-clusters.html)
 also supports to generate temporary IAM-based database user credentials.
+
+Your superset app's [IAM role should have 
permissions](https://docs.aws.amazon.com/redshift/latest/mgmt/generating-iam-credentials-role-permissions.html)
 to call the `redshift:GetClusterCredentials` operation.
+
+You have to define following arguments in Superset's redshift database 
connection UI under ADVANCED --> Others --> ENGINE PARAMETERS.
+
+```
+{"connect_args":{"iam":true,"database":"<database>","cluster_identifier":"<cluster_identifier>","db_user":"<db_user>"}}
+```
+and SQLALCHEMY URI should be set to `redshift+redshift_connector://`
+
+
+#### Using IAM-based credentials with Redshift serverless:
+
+[Redshift 
serverless](https://docs.aws.amazon.com/redshift/latest/mgmt/serverless-whatis.html)
 supports connection using IAM roles.
+
+Your superset app's IAM role should have `redshift-serverless:GetCredentials` 
and `redshift-serverless:GetWorkgroup` permissions on Redshift serverless 
workgroup.
+
+You have to define following arguments in Superset's redshift database 
connection UI under ADVANCED --> Others --> ENGINE PARAMETERS.

Review Comment:
   Missing word: define **the** following



##########
docs/docs/databases/redshift.mdx:
##########
@@ -10,6 +10,8 @@ version: 1
 The [sqlalchemy-redshift](https://pypi.org/project/sqlalchemy-redshift/) 
library is the recommended
 way to connect to Redshift through SQLAlchemy.
 
+This dialect requires either 
[redshift_connector](https://pypi.org/project/redshift-connector/) or 
[psycopg2](https://pypi.org/project/psycopg2/) to work properly.
+
 You'll need to the following setting values to form the connection string:

Review Comment:
   Missing a word between "to" and "the".
   
   Suggestion:
   You'll need to set the following values in your connection string:



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to