timsaucer commented on code in PR #1716:
URL:
https://github.com/apache/datafusion-python/pull/1716#discussion_r4025741828
##########
docs/source/user-guide/data-sources.md:
##########
@@ -100,26 +102,47 @@ Supported Object Stores are
- {py:class}`~datafusion.object_store.MicrosoftAzure`
```python
+from datafusion import SessionContext
from datafusion.object_store import AmazonS3
region = "us-east-1"
bucket_name = "yellow-trips"
+ctx = SessionContext()
+
s3 = AmazonS3(
bucket_name=bucket_name,
region=region,
- access_key_id=os.getenv("AWS_ACCESS_KEY_ID"),
- secret_access_key=os.getenv("AWS_SECRET_ACCESS_KEY"),
)
path = f"s3://{bucket_name}/"
-ctx.register_object_store("s3://", s3, None)
+ctx.register_object_store("s3://", s3, host=bucket_name)
ctx.register_parquet("trips", path)
ctx.table("trips").show()
```
+### Use S3 in SQL
+
+Use `CREATE EXTERNAL TABLE` to give an S3 path a table name that you can query
+with SQL. The statement uses the object store registered for the bucket on the
+same {py:class}`~datafusion.context.SessionContext`.
+
+After registering the object store above, create and query an external table:
Review Comment:
```suggestion
### Query S3 data with SQL
To reach the same data from SQL, give the S3 path a table name with
`CREATE EXTERNAL TABLE`. The registered object store carries the credentials
and
the region, so the statement itself needs only the location.
Register the store for the bucket as shown above, then use that same
{py:class}`~datafusion.context.SessionContext` to create and query the table:
```
##########
docs/source/user-guide/data-sources.md:
##########
@@ -100,26 +102,47 @@ Supported Object Stores are
- {py:class}`~datafusion.object_store.MicrosoftAzure`
```python
+from datafusion import SessionContext
from datafusion.object_store import AmazonS3
region = "us-east-1"
bucket_name = "yellow-trips"
+ctx = SessionContext()
+
s3 = AmazonS3(
bucket_name=bucket_name,
region=region,
- access_key_id=os.getenv("AWS_ACCESS_KEY_ID"),
- secret_access_key=os.getenv("AWS_SECRET_ACCESS_KEY"),
)
path = f"s3://{bucket_name}/"
-ctx.register_object_store("s3://", s3, None)
+ctx.register_object_store("s3://", s3, host=bucket_name)
Review Comment:
Is this change strictly necessary? I haven't tested but my agent is
suggesting:
context.rs:490 already derives `upstream_host` from `s3.bucket_name`, and
context.rs:498 only uses the argument as an override. The new form is identical
in effect, so the diff line buys nothing and invites a reader to think the host
is required for S3. If you'd rather teach the kwarg, the modern spelling is
`host=bucket_name` with the positional None gone — but then it needs a sentence
saying it's an override, which is scope creep for a docs PR. Simplest: drop the
hunk.
--
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]
