ably-python icon indicating copy to clipboard operation
ably-python copied to clipboard

Use endpoint as default connection option (ADR-119)

Open surminus opened this issue 10 months ago • 6 comments

This implements ADR-119[1], which specifies the client connection options to update requests to the endpoints implemented as part of ADR-042[2].

The endpoint may be one of the following:

a routing policy name (such as main) a nonprod routing policy name (such as nonprod:sandbox) a FQDN such as foo.example.com The endpoint option is not valid with any of environment, restHost or realtimeHost, but we still intend to support the legacy options.

If the client has been configured to use any of these legacy options, then they should continue to work in the same way, using the same primary and fallback hostnames.

If the client has not been explicitly configured, then the hostnames will change to the new ably.net domain when the package is upgraded.

Notes on implementation

As per previous guidance, I have not updated tests to use the endpoint client option, but since the hostnames are changing, then the assertions and default environment must be updated (from sandbox to nonprod:sandbox).

I have not added any explicit tests for the endpoint client option, but I can do this if we think it's necessary, although using environment is essentially the same as using endpoint.

See related Go and Ruby PRs:

  • https://github.com/ably/ably-go/pull/679
  • https://github.com/ably/ably-ruby/pull/441

[1] https://ably.atlassian.net/wiki/spaces/ENG/pages/3428810778/ADR-119+ClientOptions+for+new+DNS+structure [2] https://ably.atlassian.net/wiki/spaces/ENG/pages/1791754276/ADR-042+DNS+Restructure

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Introduced an “endpoint” parameter for more reliable connection configurations, replacing previous environment settings.
    • Updated service endpoints ensure production connections default to the new primary host while sandbox (non-production) environments use revised host URLs.
    • Added support for specifying routing policies or fully qualified domain names via the new endpoint parameter in client initialization.
  • Bug Fixes

    • Adjusted tests to align with new host configurations, ensuring accurate assertions for both production and sandbox environments.
  • Refactor

    • Streamlined configuration and validation to minimize conflicting inputs and clarify host selection, enhancing overall connectivity.
    • Deprecated older parameters to improve clarity in connection settings.

surminus avatar Feb 20 '25 16:02 surminus

Walkthrough

The pull request modifies the host configuration logic in the Ably SDK. In ably/transport/defaults.py, the Defaults class replaces static fallback hosts and hostnames with dynamic generation based on an endpoint string. The Options class in ably/types/options.py enforces mutual exclusivity between endpoint and environment parameters and uses the new hostname resolution methods. Test files are updated to reflect new host URLs and environment naming conventions. The AblyRest and AblyRealtime classes add an endpoint parameter and mark rest_host and environment as deprecated.

Changes

File(s) Change Summary
ably/transport/defaults.py Refactored Defaults class: removed static fallback hosts and hostnames; replaced environment with endpoint attribute; added static methods get_hostname and get_fallback_hosts for dynamic host generation based on endpoint string.
ably/types/options.py Updated Options constructor to add endpoint parameter; enforce mutual exclusivity with environment, rest_host, and realtime_host; replaced environment property with endpoint; updated host resolution logic to use Defaults.get_hostname and Defaults.get_fallback_hosts.
test/ably/rest/restinit_test.py, test/ably/rest/restpaginatedresult_test.py, test/ably/rest/restrequest_test.py, test/ably/testapp.py Updated test host URLs and environment identifiers to match new endpoint-based host naming conventions, including changes from "production" to "main" and "sandbox" to "nonprod:sandbox".
ably/realtime/realtime.py, ably/rest/rest.py Added new endpoint parameter to AblyRealtime and AblyRest constructors; marked existing realtime_host, rest_host, and environment parameters as deprecated in documentation.
test/unit/options_test.py Added new test suite for Options class validating mutual exclusivity of parameters, default and custom endpoint hostnames, fallback hosts, and handling of FQDN and IP addresses as endpoints.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Options
    participant Defaults
    Client->>Options: Instantiate with endpoint (or environment)
    Options->>Options: Validate mutual exclusivity (raise error if both present)
    Options->>Defaults: get_hostname(endpoint)
    Defaults-->>Options: Return primary hostname
    Options->>Defaults: get_fallback_hosts(endpoint)
    Defaults-->>Options: Return fallback hosts
    Options-->>Client: Return configured Options instance

Poem

I'm a bunny, hopping with glee,
New endpoints shine for all to see.
I nibble on bugs with whiskers twitch,
As fallback hosts appear without a glitch.
With every hop and joyful cheer,
CodeRabbit sings: "Update, my dear!"
(_/) 🥕 Happy coding!

[!NOTE]

⚡️ AI Code Reviews for VS Code, Cursor, Windsurf

CodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Learn more here.


[!NOTE]

⚡️ Faster reviews with caching

CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure Review - Disable Cache at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off the Data Retention setting under your Organization Settings. Enjoy the performance boost—your workflow just got faster.


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Feb 20 '25 16:02 coderabbitai[bot]

Great, thank you for the review.

Given the impact of this changes, I would also like to see some endpoint tests (could be simple unit tests) to check that providing a new endpoint option results in the expected rest host, realtime host and fallback hosts used.

I will add these tests.

Should we do the same here, or can create a separate ticket for it? Don't know if there was an explicit decision to deprecate those.

I can do that as part of this PR, that's fine. I'll also need to update my ably-js and ably-ruby PRs.

surminus avatar Mar 03 '25 11:03 surminus

Just a heads up:

test/ably/rest/resthttp_test.py::TestRestHttp::test_request_headers - test consistently fails after changes, should investigate

This is not related to this PR, fixed in https://github.com/ably/ably-python/pull/593. You can ignore test_request_headers test failures for the time being

VeskeR avatar Mar 04 '25 08:03 VeskeR

@VeskeR @sacOO7 thanks for reviews, I've updated this PR with a fixup that adds some (very basic) tests, ensures client combinations are incompatible, and addresses code comments.

surminus avatar Mar 11 '25 17:03 surminus

@surminus as discussed on Slack, please could you make sure that this PR implements / tests the updates to REC1b2 from https://github.com/ably/specification/pull/302? Thanks!

lawrence-forooghian avatar Apr 22 '25 19:04 lawrence-forooghian

@lawrence-forooghian updated

surminus avatar May 22 '25 11:05 surminus