Skip to content

Export generate_random_key and get_default_params from ably.pubsub.server - #733

Merged
sacOO7 merged 1 commit into
integration/v4from
feature/export-crypto-helpers
Oct 16, 2026
Merged

sacOO7 merged 1 commit into
integration/v4from
feature/export-crypto-helpers

Conversation

@sacOO7

@sacOO7 sacOO7 commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Exports generate_random_key and get_default_params from ably.pubsub.server and ably.pubsub.server.sync, so 4.0 has a public way to generate a channel encryption key (RSE2) and build default cipher params.

Why

4.0 makes ably.pubsub.server the whole public API, and the packages beneath it are internal (UPDATING.md). Its __all__ was carried over from 3.x's top-level ably/__init__.py, which only re-exported CipherParams from crypto. In 3.x, users got the key generator from the submodule instead:

from ably.util.crypto import generate_random_key  # 3.x

On 4.0 that raises ModuleNotFoundError: No module named 'ably.util'. The function still exists at ably.pubsub.util.crypto, but that path is internal, so there was no supported import. Every other Pub/Sub SDK exposes RSE2 publicly (Crypto.generateRandomKey() in Java, JS, Flutter and PHP, pubsub.Crypto.GenerateRandomKey() in Go, Crypto.GenerateRandomKey() in .NET).

This came up in the 4.0 docs review (https://github.com/ably/website/pull/10418#discussion_r4281476680), where the Python encryption examples currently fall back to secrets.token_bytes(32).

Changes

  • ably/pubsub/server/__init__.py and ably/pubsub/server/sync.py: import and export both helpers. sync.py is hand-written (unasync excludes ably/pubsub/server), so it gets the same edit.
  • ably/pubsub/util/nocrypto.py: the missing-crypto error now names pip install 'ably-pubsub-server[crypto]' instead of the 3.x ably[crypto].
  • UPDATING.md: a 3.x → 4.0.0 import example for the encryption helpers.
  • test/unit/pubsub_server_test.py: asserts both names are in __all__ of the async and sync modules.

Checks

  • uv run unasync, then pytest test/unit: 115 passed. ruff check: clean.
  • pytest test/ably/http/httpcrypto_test.py -k "Fixture or random or default" (fixtures from the test-resources submodule): 6 passed.
  • End to end, offline: from ably.pubsub.server import generate_random_key gives a 32-byte key, and publishing on a channel with cipher={'key': key} sends encoding: utf-8/cipher+aes-256-cbc/base64. The sync generate_random_key() also gives 32 bytes.

Summary by CodeRabbit

  • Documentation
    • Added a migration example showing where to import encryption helpers in version 4.0.0.
  • New Features
    • Made generate_random_key and get_default_params available through both synchronous and asynchronous server modules.
  • Bug Fixes
    • Updated the encryption dependency error message to reference the correct installation option.

4.0 makes ably.pubsub.server the whole public API, but its export list
was carried over from 3.x's top-level ably package, which only
re-exported CipherParams. 3.x users reached generate_random_key and
get_default_params through ably.util.crypto, a submodule that 4.0 moves
and declares internal, so 4.0 left no public way to generate a key
(RSE2) or build default cipher params.

Export both from ably.pubsub.server and ably.pubsub.server.sync, show the
3.x to 4.0 import in UPDATING.md, and make the missing-crypto error name
the ably-pubsub-server[crypto] extra instead of the old ably[crypto].
@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1301b6be-8a64-41a1-b5d2-4e67e3b33f22
📥 Commits

Reviewing files that changed from the base of the PR and between 4a869cf and d23c98c.

📒 Files selected for processing: 5
  • UPDATING.md
  • ably/pubsub/server/__init__.py
  • ably/pubsub/server/sync.py
  • ably/pubsub/util/nocrypto.py
  • test/unit/pubsub_server_test.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The asynchronous and synchronous server modules now export generate_random_key and get_default_params. The migration guide shows the updated import path, and the crypto installation error message names the server package extra.

Changes

Crypto helper exports

Layer / File(s) Summary
Public exports and migration guidance
ably/pubsub/server/__init__.py, ably/pubsub/server/sync.py, test/unit/pubsub_server_test.py, UPDATING.md, ably/pubsub/util/nocrypto.py
Both server modules import and export generate_random_key and get_default_params. A parametrized test checks both export lists. The migration guide shows the new import path, and the crypto installation error message names ably-pubsub-server[crypto].

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Suggested reviewers: ttypic

Merge Risk: ⚪ Minimal · up to d23c9

The change provides the new public crypto-helper imports and matching installation guidance. No actionable merge-blocking risk was identified.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the primary change: exporting generate_random_key and get_default_params from ably.pubsub.server.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 2
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks the exports in a row
Two crypto helpers join the show
The guide points to their new home
The error message tells where to roam
Then off through springtime fields I go

Comment @coderabbitai help to get the list of available commands.

@ttypic ttypic left a comment

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.

LGTM

@sacOO7
sacOO7 merged commit 0c75418 into integration/v4 Oct 16, 2026
8 of 10 checks passed
@sacOO7
sacOO7 deleted the feature/export-crypto-helpers branch October 16, 2026 08:17

This branch was successfully deployed

1 active deployment
staging/pull/733/features — d23c98c6 Deployed Oct 16, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants