Additional object fields - #1
Conversation
There was a problem hiding this comment.
Pull request overview
This PR generalizes S3 object key generation by replacing the single hostname component with a configurable set of key/value fields, while retaining backwards compatibility via S3PUSHER_HOSTNAME.
Changes:
- Replace
--hostname/hostnameusage with--fieldsand anobject_kvsdict used when building the S3 object key. - Add parsing for
S3PUSHER_FIELDSand mapS3PUSHER_HOSTNAMEintoobject_kvs["hostname"]. - Update documentation and bump tooling/dependencies in
uv.lock(plus a small VS Code Ruff setting tweak).
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
s3pusher.py |
Switches object key construction to include arbitrary fields and updates CLI/env parsing. |
README.md |
Updates usage/docs to describe --fields and new object key format. |
uv.lock |
Updates locked versions for dependencies/tools (e.g., boto3/botocore/ruff). |
.vscode/settings.json |
Enables Ruff formatter preview setting. |
Comments suppressed due to low confidence (1)
s3pusher.py:65
boto3.client("s3")is created twice inside the same upload attempt (once before the contextvars block and again inside it). This is redundant and adds overhead; create it once and reuse the same client for the upload.
s3_client = boto3.client("s3")
with structlog.contextvars.bound_contextvars(s3_bucket=self.bucket, s3_object_key=s3_object_key):
self.logger.debug("Uploading file")
s3_client = boto3.client("s3")
t1 = time.time()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Jakob Schlyter <jakob@kirei.se>
Replace
hostnameargument with a generic list of key/value pairs for S3 object names, e.g.--fields provider=xyzzy,site=got,hostname=xyzzy.example.comKeep support for environment variable
S3PUSHER_HOSTNAMEashostnamefor backwards compability.