Skip to content

Update README#33

Merged
m-szymon merged 1 commit into
scylladb:masterfrom
lukaszg22:readme
Jun 11, 2026
Merged

Update README#33
m-szymon merged 1 commit into
scylladb:masterfrom
lukaszg22:readme

Conversation

@lukaszg22

Copy link
Copy Markdown
Contributor

Add README. The glossary is mostly copied from the GoLang Alternator Client README.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a substantially expanded README for the alternator-driver crate, documenting terminology and intended usage patterns for an Alternator-compatible DynamoDB client in Rust.

Changes:

  • Replaces the placeholder README with a full crate overview and glossary.
  • Adds usage examples for configuring AlternatorConfig / AlternatorClient.
  • Documents features such as load balancing, routing scope, key route affinity, header stripping, compression, and per-operation overrides.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread README.md
Comment thread README.md
Comment thread README.md
Comment thread README.md
Comment thread README.md
Comment thread README.md Outdated
Comment on lines +293 to +299
Alternator accepts compressed request bodies, which can significantly reduce the bandwidth for write-heavy workloads (especially `BatchWriteItem` and large `PutItem` payloads).

Compression is **off by default**. To enable it, pass a `RequestCompression` configuration:

```rust
use alternator_driver::{AlternatorConfig, AlternatorClient, RequestCompression, CompressionAlgorithm, CompressionLevel};

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and we decided to change it to be off by default, but this change isn't implemented yet. I will add a note about that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe let's implement the change instead of leaving a note?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just made a PR for this #34

Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment on lines +82 to +83
## Load balancing
> **Note:** load balancing is not yet merged, the changes described here are implemented in PRs #30, #31, and #32.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ Is it going to be merged before the ZPP finishes in late June?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving the PRs opened but unmerged is rarely a good option.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I hope so. Currently we are pretty much done discussing the changes, but since the move to scylladb org @m-szymon has lost the rights to merge.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can merge them whenever you decide it's the good moment to do so.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK then #31 is ready to merge. #30 needs one change that I'm doing today and that @m-szymon would probably want to take a look at. After that, it can be merged alongside #32.

Comment thread README.md Outdated
Comment thread README.md
Comment thread README.md
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment on lines +293 to +299
Alternator accepts compressed request bodies, which can significantly reduce the bandwidth for write-heavy workloads (especially `BatchWriteItem` and large `PutItem` payloads).

Compression is **off by default**. To enable it, pass a `RequestCompression` configuration:

```rust
use alternator_driver::{AlternatorConfig, AlternatorClient, RequestCompression, CompressionAlgorithm, CompressionLevel};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe let's implement the change instead of leaving a note?

Comment thread README.md Outdated
Comment thread README.md Outdated
Comment on lines +343 to +345
`.alternator_config_override()` is the Alternator-specific equivalent of the AWS SDK's `.config_override()`. The two work alongside each other — use `.config_override()` for SDK-level settings (retry behavior, timeouts) and `.alternator_config_override()` for Alternator-specific settings.

> **Note**: load-balancing and endpoint settings cannot be overridden per-operation. They take effect only when the client is constructed. Per-operation override is for settings that apply to individual request processing — compression and header stripping. No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 So AlternatorConfig may represent invalid state when used in alternator_config_override()? This is bad design. If some options make sense in only one of {alternator_config_override(), config_override()}, then they should accept distinct configs.

If you want to avoid code duplication when separating the configs, there are ways to do that - I suggest the typestate pattern. See GenericSessionBuilder in Rust Driver as a simplified example of this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think README is a bit ambiguous, alternator_config_override is an extension of config_override.

Developer may keep config_override in their project or swap it with alternator_config_override for the same result. Swap is neccessary if they want to change an alternator-specific optimization like header stripping.

@wprzytula wprzytula May 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nonetheless, please confirm or correct my understanding:

AlternatorConfig may represent some settings that are only respected in alternator_config_override and not in config_override.

If the above is true, what I said still holds. Allowing user setting some options which are silently ignored is a footgun.

@wkkasztan wkkasztan May 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AlternatorConfig cannot be passed into config_override(override : DynamodbConfig), only into alternator_config_override(override : AlternatorConfig).

AlternatorConfig stores an inner DynamodbConfig with all of DynamoDB's settings.
And alternator_config_override calls config_override with that inner config. (Also injecting alternator-specific optimizations to that config, before it is forwarded)

Comment thread README.md Outdated
@lukaszg22 lukaszg22 marked this pull request as draft May 27, 2026 12:56
@m-szymon

m-szymon commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

@lukaszg22 @wkkasztan please update README to include #34 and #32 (if not yet done).
Resolve fixed conversations.
I will review ready version.

Comment thread README.md Outdated
Comment thread README.md
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated

@m-szymon m-szymon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few more remarks.

Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated

@m-szymon m-szymon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lukaszg22 please rebase it, squash commits (it is all a single update of README) and mark it as ready. I would like to merge this.

@lukaszg22 lukaszg22 marked this pull request as ready for review June 11, 2026 09:43
@m-szymon m-szymon dismissed wprzytula’s stale review June 11, 2026 10:12

Comments were addressed.

@m-szymon m-szymon merged commit f7cb866 into scylladb:master Jun 11, 2026
2 checks passed
@lukaszg22 lukaszg22 deleted the readme branch June 11, 2026 11:54
@stopnoanime stopnoanime mentioned this pull request Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants