Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use endpoint as default connection option (ADR-119) #590

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 31 additions & 17 deletions ably/transport/defaults.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,8 @@
class Defaults:
protocol_version = "2"
fallback_hosts = [
"a.ably-realtime.com",
"b.ably-realtime.com",
"c.ably-realtime.com",
"d.ably-realtime.com",
"e.ably-realtime.com",
]

rest_host = "rest.ably.io"
realtime_host = "realtime.ably.io" # RTN2

connectivity_check_url = "https://internet-up.ably-realtime.com/is-the-internet-up.txt"
environment = 'production'
endpoint = 'main'

port = 80
tls_port = 443
Expand Down Expand Up @@ -53,11 +44,34 @@ def get_scheme(options):
return "http"

@staticmethod
def get_environment_fallback_hosts(environment):
def get_hostname(endpoint):
if "." in endpoint or "::" in endpoint or "localhost" in endpoint:
return endpoint

if endpoint.startswith("nonprod:"):
return endpoint[len("nonprod:"):] + ".realtime.ably-nonprod.net"

if endpoint == "main":
return "main.realtime.ably.net"

return endpoint + ".realtime.ably.net"

@staticmethod
def get_fallback_hosts(endpoint="main"):
if endpoint.startswith("nonprod:"):
root = endpoint.replace("nonprod:", "")
return [
root + ".a.fallback.ably-realtime-nonprod.com",
root + ".b.fallback.ably-realtime-nonprod.com",
root + ".c.fallback.ably-realtime-nonprod.com",
root + ".d.fallback.ably-realtime-nonprod.com",
root + ".e.fallback.ably-realtime-nonprod.com",
]

return [
environment + "-a-fallback.ably-realtime.com",
environment + "-b-fallback.ably-realtime.com",
environment + "-c-fallback.ably-realtime.com",
environment + "-d-fallback.ably-realtime.com",
environment + "-e-fallback.ably-realtime.com",
endpoint + ".a.fallback.ably-realtime.com",
endpoint + ".b.fallback.ably-realtime.com",
endpoint + ".c.fallback.ably-realtime.com",
endpoint + ".d.fallback.ably-realtime.com",
endpoint + ".e.fallback.ably-realtime.com",
]
51 changes: 25 additions & 26 deletions ably/types/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@

class Options(AuthOptions):
def __init__(self, client_id=None, log_level=0, tls=True, rest_host=None, realtime_host=None, port=0,
tls_port=0, use_binary_protocol=True, queue_messages=False, recover=False, environment=None,
http_open_timeout=None, http_request_timeout=None, realtime_request_timeout=None,
http_max_retry_count=None, http_max_retry_duration=None, fallback_hosts=None,
fallback_retry_timeout=None, disconnected_retry_timeout=None, idempotent_rest_publishing=None,
loop=None, auto_connect=True, suspended_retry_timeout=None, connectivity_check_url=None,
tls_port=0, use_binary_protocol=True, queue_messages=False, recover=False, endpoint=None,
environment=None, http_open_timeout=None, http_request_timeout=None,
realtime_request_timeout=None, http_max_retry_count=None, http_max_retry_duration=None,
fallback_hosts=None, fallback_retry_timeout=None, disconnected_retry_timeout=None,
idempotent_rest_publishing=None, loop=None, auto_connect=True,
suspended_retry_timeout=None, connectivity_check_url=None,
channel_retry_timeout=Defaults.channel_retry_timeout, add_request_ids=False, **kwargs):

super().__init__(**kwargs)
Expand Down Expand Up @@ -42,12 +43,21 @@ def __init__(self, client_id=None, log_level=0, tls=True, rest_host=None, realti
if environment is not None and realtime_host is not None:
raise ValueError('specify realtime_host or environment, not both')

if environment is not None and endpoint is not None:
raise ValueError('specify endpoint or environment, not both')

if idempotent_rest_publishing is None:
from ably import api_version
idempotent_rest_publishing = api_version >= '1.2'

if environment is None:
environment = Defaults.environment
if environment == "production":
endpoint = Defaults.endpoint

if environment is not None and endpoint is None:
endpoint = environment

if endpoint is None:
endpoint = Defaults.endpoint

self.__client_id = client_id
self.__log_level = log_level
Expand All @@ -59,7 +69,7 @@ def __init__(self, client_id=None, log_level=0, tls=True, rest_host=None, realti
self.__use_binary_protocol = use_binary_protocol
self.__queue_messages = queue_messages
self.__recover = recover
self.__environment = environment
self.__endpoint = endpoint
self.__http_open_timeout = http_open_timeout
self.__http_request_timeout = http_request_timeout
self.__realtime_request_timeout = realtime_request_timeout
Expand Down Expand Up @@ -163,8 +173,8 @@ def recover(self, value):
self.__recover = value

@property
def environment(self):
return self.__environment
def endpoint(self):
return self.__endpoint

@property
def http_open_timeout(self):
Expand Down Expand Up @@ -268,27 +278,19 @@ def __get_rest_hosts(self):
# Defaults
host = self.rest_host
if host is None:
host = Defaults.rest_host

environment = self.environment
host = Defaults.get_hostname(self.endpoint)

http_max_retry_count = self.http_max_retry_count
if http_max_retry_count is None:
http_max_retry_count = Defaults.http_max_retry_count

# Prepend environment
if environment != 'production':
host = '%s-%s' % (environment, host)

# Fallback hosts
fallback_hosts = self.fallback_hosts
if fallback_hosts is None:
if host == Defaults.rest_host:
fallback_hosts = Defaults.fallback_hosts
elif environment != 'production':
fallback_hosts = Defaults.get_environment_fallback_hosts(environment)
else:
if self.rest_host:
fallback_hosts = []
else:
fallback_hosts = Defaults.get_fallback_hosts(self.endpoint)

# Shuffle
fallback_hosts = list(fallback_hosts)
Expand All @@ -304,11 +306,8 @@ def __get_realtime_hosts(self):
if self.realtime_host is not None:
host = self.realtime_host
return [host]
elif self.environment != "production":
host = f'{self.environment}-{Defaults.realtime_host}'
else:
host = Defaults.realtime_host

host = Defaults.get_hostname(self.endpoint)
return [host] + self.__fallback_hosts

def get_rest_hosts(self):
Expand Down
24 changes: 15 additions & 9 deletions test/ably/rest/restinit_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,12 @@ def test_rest_host_and_environment(self):
# environment: production
ably = AblyRest(token='foo', environment="production")
host = ably.options.get_rest_host()
assert "rest.ably.io" == host, "Unexpected host mismatch %s" % host
assert "main.realtime.ably.net" == host, "Unexpected host mismatch %s" % host

# environment: other
ably = AblyRest(token='foo', environment="sandbox")
ably = AblyRest(token='foo', environment="nonprod:sandbox")
host = ably.options.get_rest_host()
assert "sandbox-rest.ably.io" == host, "Unexpected host mismatch %s" % host
assert "sandbox.realtime.ably-nonprod.net" == host, "Unexpected host mismatch %s" % host
Comment on lines +82 to +84
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix Yoda condition and update environment to endpoint.

Update the test to use endpoint instead of environment for consistency with ADR-119.

-        ably = AblyRest(token='foo', environment="nonprod:sandbox")
+        ably = AblyRest(token='foo', endpoint="nonprod:sandbox")
         host = ably.options.get_rest_host()
-        assert "sandbox.realtime.ably-nonprod.net" == host, "Unexpected host mismatch %s" % host
+        assert host == "sandbox.realtime.ably-nonprod.net", "Unexpected host mismatch %s" % host
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ably = AblyRest(token='foo', environment="nonprod:sandbox")
host = ably.options.get_rest_host()
assert "sandbox-rest.ably.io" == host, "Unexpected host mismatch %s" % host
assert "sandbox.realtime.ably-nonprod.net" == host, "Unexpected host mismatch %s" % host
ably = AblyRest(token='foo', endpoint="nonprod:sandbox")
host = ably.options.get_rest_host()
assert host == "sandbox.realtime.ably-nonprod.net", "Unexpected host mismatch %s" % host
🧰 Tools
🪛 Ruff (0.8.2)

84-84: Yoda condition detected

Rewrite as host == "sandbox.realtime.ably-nonprod.net"

(SIM300)


# both, as per #TO3k2
with pytest.raises(ValueError):
Expand All @@ -103,13 +103,13 @@ def test_fallback_hosts(self):
assert sorted(aux) == sorted(ably.options.get_fallback_rest_hosts())

# Specify environment (RSC15g2)
ably = AblyRest(token='foo', environment='sandbox', http_max_retry_count=10)
assert sorted(Defaults.get_environment_fallback_hosts('sandbox')) == sorted(
ably = AblyRest(token='foo', environment='nonprod:sandbox', http_max_retry_count=10)
assert sorted(Defaults.get_fallback_hosts('nonprod:sandbox')) == sorted(
ably.options.get_fallback_rest_hosts())

# Fallback hosts and environment not specified (RSC15g3)
ably = AblyRest(token='foo', http_max_retry_count=10)
assert sorted(Defaults.fallback_hosts) == sorted(ably.options.get_fallback_rest_hosts())
assert sorted(Defaults.get_fallback_hosts()) == sorted(ably.options.get_fallback_rest_hosts())
Comment on lines +106 to +112
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Update environment to endpoint in fallback hosts test.

For consistency with ADR-119, update the test to use endpoint instead of environment.

-        ably = AblyRest(token='foo', environment='nonprod:sandbox', http_max_retry_count=10)
+        ably = AblyRest(token='foo', endpoint='nonprod:sandbox', http_max_retry_count=10)
         assert sorted(Defaults.get_fallback_hosts('nonprod:sandbox')) == sorted(
             ably.options.get_fallback_rest_hosts())
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ably = AblyRest(token='foo', environment='nonprod:sandbox', http_max_retry_count=10)
assert sorted(Defaults.get_fallback_hosts('nonprod:sandbox')) == sorted(
ably.options.get_fallback_rest_hosts())
# Fallback hosts and environment not specified (RSC15g3)
ably = AblyRest(token='foo', http_max_retry_count=10)
assert sorted(Defaults.fallback_hosts) == sorted(ably.options.get_fallback_rest_hosts())
assert sorted(Defaults.get_fallback_hosts()) == sorted(ably.options.get_fallback_rest_hosts())
ably = AblyRest(token='foo', endpoint='nonprod:sandbox', http_max_retry_count=10)
assert sorted(Defaults.get_fallback_hosts('nonprod:sandbox')) == sorted(
ably.options.get_fallback_rest_hosts())
# Fallback hosts and environment not specified (RSC15g3)
ably = AblyRest(token='foo', http_max_retry_count=10)
assert sorted(Defaults.get_fallback_hosts()) == sorted(ably.options.get_fallback_rest_hosts())


# RSC15f
ably = AblyRest(token='foo')
Expand Down Expand Up @@ -182,13 +182,19 @@ async def test_query_time_param(self):
@dont_vary_protocol
def test_requests_over_https_production(self):
ably = AblyRest(token='token')
assert 'https://rest.ably.io' == '{0}://{1}'.format(ably.http.preferred_scheme, ably.http.preferred_host)
assert 'https://main.realtime.ably.net' == '{0}://{1}'.format(
ably.http.preferred_scheme, ably.http.preferred_host
)

assert ably.http.preferred_port == 443

@dont_vary_protocol
def test_requests_over_http_production(self):
ably = AblyRest(token='token', tls=False)
assert 'http://rest.ably.io' == '{0}://{1}'.format(ably.http.preferred_scheme, ably.http.preferred_host)
assert 'http://main.realtime.ably.net' == '{0}://{1}'.format(
ably.http.preferred_scheme, ably.http.preferred_host
)

assert ably.http.preferred_port == 80

@dont_vary_protocol
Expand All @@ -211,7 +217,7 @@ async def test_environment(self):
except AblyException:
pass
request = get_mock.call_args_list[0][0][0]
assert request.url == 'https://custom-rest.ably.io:443/time'
assert request.url == 'https://custom.realtime.ably.net:443/time'

await ably.close()

Expand Down
10 changes: 5 additions & 5 deletions test/ably/rest/restpaginatedresult_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ async def asyncSetUp(self):
self.ably = await TestApp.get_ably_rest(use_binary_protocol=False)
# Mocked responses
# without specific headers
self.mocked_api = respx.mock(base_url='http://rest.ably.io')
self.mocked_api = respx.mock(base_url='http://main.realtime.ably.net')
self.ch1_route = self.mocked_api.get('/channels/channel_name/ch1')
self.ch1_route.return_value = Response(
headers={'content-type': 'application/json'},
Expand All @@ -44,8 +44,8 @@ async def asyncSetUp(self):
headers={
'content-type': 'application/json',
'link':
'<http://rest.ably.io/channels/channel_name/ch2?page=1>; rel="first",'
' <http://rest.ably.io/channels/channel_name/ch2?page=2>; rel="next"'
'<http://main.realtime.ably.net/channels/channel_name/ch2?page=1>; rel="first",'
' <http://main.realtime.ably.net/channels/channel_name/ch2?page=2>; rel="next"'
},
body='[{"id": 0}, {"id": 1}]',
status=200
Expand All @@ -55,11 +55,11 @@ async def asyncSetUp(self):

self.paginated_result = await PaginatedResult.paginated_query(
self.ably.http,
url='http://rest.ably.io/channels/channel_name/ch1',
url='http://main.realtime.ably.net/channels/channel_name/ch1',
response_processor=lambda response: response.to_native())
self.paginated_result_with_headers = await PaginatedResult.paginated_query(
self.ably.http,
url='http://rest.ably.io/channels/channel_name/ch2',
url='http://main.realtime.ably.net/channels/channel_name/ch2',
response_processor=lambda response: response.to_native())

async def asyncTearDown(self):
Expand Down
4 changes: 2 additions & 2 deletions test/ably/rest/restrequest_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ async def test_timeout(self):
await ably.request('GET', '/time', version=Defaults.protocol_version)
await ably.close()

default_endpoint = 'https://sandbox-rest.ably.io/time'
fallback_host = 'sandbox-a-fallback.ably-realtime.com'
default_endpoint = 'https://sandbox.realtime.ably-nonprod.net/time'
fallback_host = 'sandbox.a.fallback.ably-realtime-nonprod.com'
fallback_endpoint = f'https://{fallback_host}/time'
ably = await TestApp.get_ably_rest(fallback_hosts=[fallback_host])
with respx.mock:
Expand Down
8 changes: 4 additions & 4 deletions test/ably/testapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
app_spec_local = json.loads(f.read())

tls = (os.environ.get('ABLY_TLS') or "true").lower() == "true"
rest_host = os.environ.get('ABLY_REST_HOST', 'sandbox-rest.ably.io')
realtime_host = os.environ.get('ABLY_REALTIME_HOST', 'sandbox-realtime.ably.io')
rest_host = os.environ.get('ABLY_REST_HOST', 'sandbox.realtime.ably-nonprod.net')
realtime_host = os.environ.get('ABLY_REALTIME_HOST', 'sandbox.realtime.ably-nonprod.net')

environment = os.environ.get('ABLY_ENV', 'sandbox')
environment = os.environ.get('ABLY_ENV', 'nonprod:sandbox')

port = 80
tls_port = 443

if rest_host and not rest_host.endswith("rest.ably.io"):
if rest_host and not rest_host.endswith("realtime.ably-nonprod.net"):
tls = tls and rest_host != "localhost"
port = 8080
tls_port = 8081
Expand Down
Loading