Skip to content

feat: rest layered#5220

Open
seanpearsonuk wants to merge 137 commits into
mainfrom
feat/rest-layered
Open

feat: rest layered#5220
seanpearsonuk wants to merge 137 commits into
mainfrom
feat/rest-layered

Conversation

@seanpearsonuk

Copy link
Copy Markdown
Collaborator

REST Client Refactoring: Layered Architecture with Injectable Transport Strategy

Summary

The original rest/client.py was a single 415-line file that handled four
distinct responsibilities at once: error modelling, HTTP transport mechanics,
authentication, and the Fluent DataModel settings API. This PR splits those
responsibilities across three focused modules and introduces a strategy
interface
that makes the API layer independently unit-testable without a
running Fluent server and without patching Python's standard library.


Table of Contents

  1. The Problem: One File Doing Too Much
  2. Why That Matters
  3. The New Module Structure
  4. Design Decisions — Detailed Rationale
  5. File-by-File Breakdown
  6. How to Write Unit Tests
  7. Backwards Compatibility
  8. Dependency Graph

1. The Problem: One File Doing Too Much

The original client.py contained four interleaved layers of code:

client.py (415 lines)
├── FluentRestError          — error type and transport-exception parsing
├── _RETRYABLE_STATUS_CODES  — which HTTP codes warrant a retry
├── _RETRYABLE_METHODS       — which HTTP verbs can be retried
│
├── FluentRestClient.__init__   — accepted url, auth_token, timeout, retries, SSL…
├── _make_auth_headers          — SHA-256 hashing of bearer token
├── _build_request              — assembled urllib.request.Request objects
├── _send_once                  — opened the HTTP connection and read the body
├── _send                       — caught OSError and re-raised as FluentRestError
├── _back_off                   — exponential sleep between retries
├── _send_with_retry            — retry loop
├── _request                    — entry point tying all the above together
│
├── get_static_info, get_var, get_attrs…   — Fluent settings API
├── create, delete, rename…                — named-object CRUD
├── execute_cmd, execute_query…            — command execution
│
├── exit, __enter__, __exit__  — Python session lifecycle
└── _names_from, _size_from    — response-shape adapters

Every one of those items lived in a single class.


2. Why That Matters

It is hard to test

To test whether delete(..., ignore_not_found=True) correctly swallows a 404
but still re-raises a 500, you would previously have needed to either:

  • spin up a real Fluent server, or
  • mock Python's urllib.request.urlopen at a very low level — a fragile
    approach that breaks if the internal implementation changes.

The API logic (what the method does with the response) and the transport
mechanics (how the HTTP call is made) were fused together, so you could not
test one without the other.

It is hard to reason about

When reading the retry logic, you have to hold in your head that it sits
inside the same class as get_var and create. When reading get_var,
you have to remember that self._request quietly does retries, checks a
closed-session flag, builds the URL, and SHA-256-hashes your token before
touching the network. That mental overhead accumulates.

The closed-session guard was misplaced

def _request(self, method, endpoint, *, body=None):
	if self._is_closed:
		raise FluentRestError(0, "Session is closed")   # ← raw details at call site
	...

The caller of _request (i.e., every API method) had to silently rely on the
fact that an internal flag was checked. The error constructor arguments
(0, "Session is closed") were copy-pasted knowledge rather than a named
concept.

Error construction was fragile

# Before — scattered across the codebase, easy to get wrong:
raise FluentRestError(0, "Session is closed")
raise FluentRestError(exc.code, body or exc.reason, retryable=...)

The numeric status code 0 as a sentinel for "no network connection" was an
implicit convention, not an enforced one.


3. The New Module Structure

src/ansys/fluent/core/rest/
├── __init__.py       ← public surface, unchanged imports for callers
├── errors.py         ← FluentRestError and everything it needs to know
├── transport.py      ← RequestStrategy protocol + HttpRequestStrategy
└── client.py         ← FluentRestClient: pure Fluent API, no HTTP code

rest_connect.py has been deleted — its single function is now the
FluentRestClient.connect() classmethod (see §4.5).

Line counts after refactoring:

File Lines Responsibility
errors.py 79 Error types only
transport.py 183 HTTP transport only
client.py 290 Fluent API only
__init__.py 63 Public surface only

4. Design Decisions — Detailed Rationale

4.1 errors.py — one home for all error knowledge

Before:

# client.py
_RETRYABLE_STATUS_CODES = frozenset({502, 503, 504})

class FluentRestError(RuntimeError):
	def __init__(self, status: int, message: str, *, retryable: bool = False):
		...

	@classmethod
	def from_transport(cls, exc: OSError) -> "FluentRestError":
		...
		return cls(exc.code, message, retryable=exc.code in _RETRYABLE_STATUS_CODES)

After:

# errors.py
_RETRYABLE_STATUS_CODES = frozenset({502, 503, 504})   # ← moved here: only used here

class FluentRestError(RuntimeError):
	def __init__(self, status: int, message: str, *, retryable: bool = False): ...

	@classmethod
	def from_transport(cls, exc: OSError) -> "FluentRestError": ...

	@classmethod
	def not_found(cls, resource: str) -> "FluentRestError":
		return cls(404, f"Not found: {resource}")

Two things changed:

  1. _RETRYABLE_STATUS_CODES moved from client.py to errors.py. It was
    always used only by FluentRestError.from_transport — placing it next to
    that method means the two pieces of related knowledge live together.
    A future maintainer changing which codes are retryable will find the
    constant in the file dedicated to error types, not buried in the API client.

  2. Named factory methods replace raw constructor calls. Instead of every
    call site knowing that status=0 means "no connection" and typing out the
    exact string "Session is closed", each distinct error shape has one named
    constructor method. This is not merely stylistic: it means the error's
    meaning is encoded in the code, not in a magic number and an English string
    that could silently diverge.

4.2 transport.py — isolating HTTP mechanics

Before: the HTTP mechanics were methods on FluentRestClient, interspersed
with the Fluent API methods. To follow the retry logic you had to navigate
between _request, _send_with_retry, _back_off, _send, _send_once,
and _build_request while mentally filtering out the unrelated get_var and
delete methods around them.

After: HttpRequestStrategy contains all five HTTP-level steps and nothing
else:

class HttpRequestStrategy:
	#  step 1 — assemble the urllib Request object
	def _build_request(self, method, url, body): ...

	#  step 2 — open the connection, read the body, decode JSON
	def _send_once(self, req): ...

	#  step 3 — translate OSError → FluentRestError
	def _send(self, req): ...

	#  step 4 — exponential sleep
	def _back_off(self, attempt): ...

	#  step 5 — retry loop
	def _send_with_retry(self, req, retries): ...

	#  public entry point — URL construction + strategy dispatch
	def request(self, method, endpoint, *, body=None): ...

All the constructor arguments that belonged to the transport (url, auth_token,
timeout, max_retries, retry_delay, ssl_context) now live on
HttpRequestStrategy.__init__, not on FluentRestClient.__init__.
FluentRestClient no longer knows that HTTPS exists.

_make_auth_headers — previously a @staticmethod on FluentRestClient
is now a module-level private function in transport.py. It was always only
called once, from HttpRequestStrategy.__init__, so tying it to the API
client class was misleading.

4.3 The RequestStrategy Protocol — the key to testability

This is the most important change in the PR.

A Protocol in Python is a way of saying: "anything that has these
methods, in this shape, counts as satisfying this contract."
Unlike a
traditional base class, you do not have to explicitly inherit from the Protocol.
If your object has the right method signatures, it qualifies automatically.
This is called structural subtyping or duck typing with a formal contract.

from typing import Protocol, runtime_checkable

@runtime_checkable
class RequestStrategy(Protocol):
	def request(self, method: str, endpoint: str, *, body: Any = None) -> Any:
		...

FluentRestClient now declares that it needs any RequestStrategy:

class FluentRestClient:
	def __init__(self, strategy: RequestStrategy, *, component: str = "fluent_1"):
		self._strategy = strategy

In production you pass an HttpRequestStrategy:

client = FluentRestClient(HttpRequestStrategy("http://...", auth_token="secret"))

In tests you pass anything with a matching request method:

class FakeStrategy:
	def request(self, method, endpoint, *, body=None):
		return {"enabled": True}

client = FluentRestClient(FakeStrategy())

The FakeStrategy does not import urllib, does not touch the network,
does not need a Fluent license, and takes zero milliseconds to "respond."

Yet because FluentRestClient only ever calls self._strategy.request(...),
the API layer runs exactly as it would in production.

The @runtime_checkable decorator means you can use isinstance() in tests
to assert that your fake satisfies the contract:

assert isinstance(FakeStrategy(), RequestStrategy)   # True — structural match

4.4 client.py — pure API, no HTTP

FluentRestClient is now entirely free of HTTP knowledge. Every method
simply constructs the correct endpoint string for that operation and delegates
to self._strategy.request(...).

Before (_request called directly):

def get_var(self, path: str) -> Any:
	return self._request("POST", f"{self._api_base}/get_var",
						 body={"path": path.lstrip("/")})

After (strategy called directly):

def get_var(self, path: str) -> Any:
	return self._strategy.request("POST", f"{self._api_base}/get_var",
								  body={"path": path.lstrip("/")})

The visible difference is small, but the consequence is large: _request
was an internal method that mingled transport concerns; self._strategy is an
injected object with a documented interface. Any reader who wonders "what
does request actually do?" can find the answer in transport.py — or, when
writing a test, can supply their own answer.

4.5 FluentRestClient.connect() — replacing rest_connect.py

rest_connect.py contained one function:

def connect_to_webserver(url: str, auth_token: str) -> FluentRestClient:
	logger.info("Connecting to Fluent REST server at %s", url)
	return FluentRestClient(url, auth_token=auth_token)

After the refactor, constructing a production client requires assembling two
objects (HttpRequestStrategy + FluentRestClient). Placing that assembly
logic in a classmethod keeps it discoverable and co-located with the class it
produces:

class FluentRestClient:
	@classmethod
	def connect(cls, url, auth_token, *, component="fluent_1",
				timeout=30.0, max_retries=2, retry_delay=1.0,
				ssl_context=None) -> "FluentRestClient":
		logger.info("Connecting to Fluent REST server at %s", url)
		strategy = HttpRequestStrategy(url, auth_token=auth_token,
									   timeout=timeout, ...)
		return cls(strategy, component=component)

rest_connect.py is deleted. For backwards compatibility,
connect_to_webserver is re-exported from __init__.py as a plain alias:

connect_to_webserver = FluentRestClient.connect

Existing callers of connect_to_webserver(url, auth_token) continue to work
without any changes.

4.6 _names_from / _size_from — promoted to module-level functions

Before: private @staticmethods on FluentRestClient.

@staticmethod
def _names_from(result: Any) -> list[str]: ...

After: private module-level functions in client.py.

def _names_from(result: Any) -> list[str]: ...

These are pure functions — they take a value, return a value, and have no
side effects or dependencies. Attaching them to the class implied they needed
access to self, which they never did. As module-level functions they can be
imported and tested in complete isolation, without instantiating any class at
all.

4.7 Session lifecycle — removed

The original FluentRestClient included:

def exit(self) -> None:
	if self._is_closed:
		return
	self._request("POST", "api/app/exit")
	self._is_closed = True

def __enter__(self): return self
def __exit__(self, ...): self.exit()

These were removed for two reasons:

  1. exit() is just an API call. Sending POST /api/app/exit is no
    different in kind from calling execute_cmd("app", "exit"). Elevating it
    to a special lifecycle method with state management (_is_closed) implied
    a privileged status it does not actually have.

  2. _is_closed leaked session-management concerns into the transport.
    The closed-session guard (if self._is_closed: raise FluentRestError(0, "Session is closed")) was checked in _request, meaning the transport
    layer was aware of application-level session state. These are different
    layers. The caller — not the client — is responsible for knowing when to
    stop making requests.


5. File-by-File Breakdown

rest/errors.py (new file)

Item Note
_RETRYABLE_STATUS_CODES Moved from client.py; belongs with its sole consumer from_transport
FluentRestError.__init__ Unchanged
FluentRestError.from_transport() Moved from client.py; unchanged
FluentRestError.not_found() New named factory for 404 errors

rest/transport.py (new file)

Item Note
_RETRYABLE_METHODS Moved from client.py
RequestStrategy New @runtime_checkable Protocol
_make_auth_headers() Moved from FluentRestClient._make_auth_headers staticmethod
HttpRequestStrategy Extracted from FluentRestClient; owns all 6 HTTP methods

rest/client.py (rewritten)

Item Note
FluentRestClient.__init__ Now takes strategy: RequestStrategy only
FluentRestClient.connect() New classmethod; replaces rest_connect.py
All API methods Unchanged logic; self._request(...)self._strategy.request(...)
_names_from() / _size_from() Demoted from @staticmethod to module-level functions
exit, __enter__, __exit__, _is_closed Removed
_make_auth_headers, _build_request, _send_once, _send, _back_off, _send_with_retry, _request Removed (moved to transport.py)

rest/__init__.py (updated)

Item Note
FluentRestClient Re-exported; unchanged for callers
FluentRestError Now imported from errors.py directly
HttpRequestStrategy Newly exported — needed to create production clients manually
RequestStrategy Newly exported — needed to type-annotate test doubles
connect_to_webserver Alias for FluentRestClient.connect; backwards compatible

rest/rest_connect.py (deleted)

The module's single function is superseded by FluentRestClient.connect().


6. How to Write Unit Tests

The refactoring was motivated specifically by the desire to make the API layer
unit-testable. This section describes the concrete testing patterns it
enables.

6.1 The core idea: FakeStrategy

A fake (sometimes called a test double or stub) is a minimal
replacement object used in tests. It behaves enough like the real thing to
let the code under test run, but it does not do any real work — in this case,
it never opens a network connection.

The simplest possible fake is just a class with one method:

class FakeStrategy:
	def request(self, method: str, endpoint: str, *, body=None):
		return None

That is all FluentRestClient ever asks of its strategy. The protocol check
can be verified:

from ansys.fluent.core.rest.transport import RequestStrategy

assert isinstance(FakeStrategy(), RequestStrategy)

In practice, fakes are more interesting — they record calls or return
pre-programmed responses:

class FakeStrategy:
	"""Records every call and returns pre-configured responses."""

	def __init__(self, responses: dict):
		# Map of (method, endpoint) → return value
		self._responses = responses
		self.calls: list[dict] = []

	def request(self, method: str, endpoint: str, *, body=None):
		self.calls.append({"method": method, "endpoint": endpoint, "body": body})
		key = (method.upper(), endpoint)
		if key not in self._responses:
			raise KeyError(f"FakeStrategy: unexpected call {key}")
		return self._responses[key]

This fake lets tests assert what was sent to the server as well as
controlling what the server returns.

6.2 Testing individual API methods

get_var

# tests/test_rest_client.py
import pytest
from ansys.fluent.core.rest.client import FluentRestClient
from ansys.fluent.core.rest.errors import FluentRestError


class FakeStrategy:
	def __init__(self, responses):
		self._responses = responses
		self.calls = []

	def request(self, method, endpoint, *, body=None):
		self.calls.append({"method": method, "endpoint": endpoint, "body": body})
		return self._responses[(method.upper(), endpoint)]


def test_get_var_sends_correct_endpoint_and_body():
	fake = FakeStrategy({
		("POST", "api/fluent_1/get_var"): True,
	})
	client = FluentRestClient(fake)

	result = client.get_var("/setup/models/energy/enabled")

	assert result is True
	assert len(fake.calls) == 1
	call = fake.calls[0]
	assert call["method"] == "POST"
	assert call["endpoint"] == "api/fluent_1/get_var"
	# Leading slash must be stripped before sending
	assert call["body"] == {"path": "setup/models/energy/enabled"}


def test_get_var_strips_leading_slash():
	"""get_var must normalise paths with and without leading slash identically."""
	responses = {("POST", "api/fluent_1/get_var"): 42}
	fake_with = FakeStrategy(responses)
	fake_without = FakeStrategy(responses)

	result_with    = FluentRestClient(fake_with).get_var("/some/path")
	result_without = FluentRestClient(fake_without).get_var("some/path")

	assert result_with == result_without == 42
	assert fake_with.calls[0]["body"] == fake_without.calls[0]["body"]

delete with ignore_not_found

def make_error(status: int) -> FluentRestError:
	return FluentRestError(status, "error")


class ErrorStrategy:
	"""Always raises a FluentRestError with the configured status."""
	def __init__(self, status: int):
		self._error = make_error(status)

	def request(self, method, endpoint, *, body=None):
		raise self._error


def test_delete_ignores_404_when_flag_set():
	client = FluentRestClient(ErrorStrategy(404))
	# Should not raise
	client.delete("boundary/wall", "inlet", ignore_not_found=True)


def test_delete_propagates_404_when_flag_not_set():
	client = FluentRestClient(ErrorStrategy(404))
	with pytest.raises(FluentRestError) as exc_info:
		client.delete("boundary/wall", "inlet")
	assert exc_info.value.status == 404


def test_delete_propagates_500_even_when_flag_set():
	client = FluentRestClient(ErrorStrategy(500))
	with pytest.raises(FluentRestError) as exc_info:
		client.delete("boundary/wall", "inlet", ignore_not_found=True)
	assert exc_info.value.status == 500

get_attrs — URL encoding

def test_get_attrs_encodes_recursive_flag():
	fake = FakeStrategy({
		("GET", "api/fluent_1/setup/models?attrs=allowed_values,enabled&recursive=true"):
			{"allowed_values": [1, 2], "enabled": True},
	})
	client = FluentRestClient(fake)

	result = client.get_attrs(
		"setup/models", ["allowed_values", "enabled"], recursive=True
	)

	assert result == {"allowed_values": [1, 2], "enabled": True}


def test_get_attrs_omits_recursive_by_default():
	captured = {}

	class CapturingFake:
		def request(self, method, endpoint, *, body=None):
			captured["endpoint"] = endpoint
			return {}

	FluentRestClient(CapturingFake()).get_attrs("setup/models", ["enabled"])
	assert "recursive" not in captured["endpoint"]

delete_all_child_objects — composition of calls

def test_delete_all_child_objects_deletes_each_name():
	calls = []

	class RecordingFake:
		def request(self, method, endpoint, *, body=None):
			calls.append((method, endpoint))
			if method == "GET":
				return ["inlet", "outlet", "wall"]   # names returned by server
			return None   # DELETE responses

	client = FluentRestClient(RecordingFake())
	client.delete_all_child_objects("boundary", "wall")

	get_calls    = [(m, e) for m, e in calls if m == "GET"]
	delete_calls = [(m, e) for m, e in calls if m == "DELETE"]

	assert len(get_calls) == 1
	assert len(delete_calls) == 3
	deleted_names = [e.split("/")[-1] for _, e in delete_calls]
	assert set(deleted_names) == {"inlet", "outlet", "wall"}

execute_cmd?force=true flag

def test_execute_cmd_appends_force_by_default():
	captured = {}

	class CapturingFake:
		def request(self, method, endpoint, *, body=None):
			captured["endpoint"] = endpoint
			return {}

	FluentRestClient(CapturingFake()).execute_cmd("mesh", "check")
	assert captured["endpoint"].endswith("?force=true")


def test_execute_cmd_omits_force_when_disabled():
	captured = {}

	class CapturingFake:
		def request(self, method, endpoint, *, body=None):
			captured["endpoint"] = endpoint
			return {}

	FluentRestClient(CapturingFake()).execute_cmd("mesh", "check", force=False)
	assert "force" not in captured["endpoint"]

Component name is honoured

def test_component_name_is_included_in_every_endpoint():
	captured = []

	class CapturingFake:
		def request(self, method, endpoint, *, body=None):
			captured.append(endpoint)
			return {}

	meshing_client = FluentRestClient(CapturingFake(), component="fluent_meshing_1")
	meshing_client.get_var("setup/models/energy/enabled")

	assert captured[0].startswith("api/fluent_meshing_1/")

6.3 Testing error handling in the API layer

def test_rename_propagates_transport_error():
	class AlwaysFailFake:
		def request(self, method, endpoint, *, body=None):
			raise FluentRestError(503, "Service Unavailable", retryable=True)

	client = FluentRestClient(AlwaysFailFake())
	with pytest.raises(FluentRestError) as exc_info:
		client.rename("boundary/wall", new="outlet", old="inlet")
	assert exc_info.value.status == 503

6.4 Testing the transport layer in isolation

HttpRequestStrategy can be tested independently of FluentRestClient,
focusing purely on retry logic and authentication:

import unittest.mock as mock
from ansys.fluent.core.rest.transport import HttpRequestStrategy
from ansys.fluent.core.rest.errors import FluentRestError


def test_retry_logic_retries_on_retryable_error(monkeypatch):
	attempt_count = 0

	def fake_urlopen(req, timeout, context):
		nonlocal attempt_count
		attempt_count += 1
		if attempt_count < 3:
			raise FluentRestError(503, "unavailable", retryable=True)
		# Return a mock response on the third attempt
		resp = mock.MagicMock()
		resp.__enter__ = lambda s: s
		resp.__exit__ = mock.MagicMock(return_value=False)
		resp.read.return_value = b'{"ok": true}'
		return resp

	strategy = HttpRequestStrategy(
		"http://localhost:5000",
		max_retries=3,
		retry_delay=0,   # no sleeping in tests
	)

	monkeypatch.setattr("urllib.request.urlopen", fake_urlopen)
	# Note: patching urllib here is appropriate because we are specifically
	# testing the transport layer's retry mechanics, not the API layer.
	result = strategy.request("GET", "api/fluent_1/static-info")
	assert result == {"ok": True}
	assert attempt_count == 3


def test_auth_header_is_sha256_of_token():
	import hashlib
	strategy = HttpRequestStrategy("http://localhost", auth_token="my-secret")
	expected_hash = hashlib.sha256(b"my-secret").hexdigest()
	assert strategy._headers == {"Authorization": f"Bearer {expected_hash}"}


def test_no_auth_header_when_no_token():
	strategy = HttpRequestStrategy("http://localhost")
	assert strategy._headers == {}

6.5 Testing FluentRestError factories

from ansys.fluent.core.rest.errors import FluentRestError
import urllib.error, io


def test_from_transport_reads_http_error_body():
	body = b"Solver is busy"
	http_err = urllib.error.HTTPError(
		url="http://x", code=409, msg="Conflict",
		hdrs=None, fp=io.BytesIO(body)
	)
	err = FluentRestError.from_transport(http_err)
	assert err.status == 409
	assert "Solver is busy" in str(err)
	assert err.retryable is False


def test_from_transport_marks_502_as_retryable():
	http_err = urllib.error.HTTPError(
		url="http://x", code=502, msg="Bad Gateway",
		hdrs=None, fp=io.BytesIO(b"")
	)
	err = FluentRestError.from_transport(http_err)
	assert err.retryable is True


def test_from_transport_marks_connection_error_as_retryable():
	os_err = OSError("Connection refused")
	os_err.reason = "Connection refused"
	err = FluentRestError.from_transport(os_err)
	assert err.status == 0
	assert err.retryable is True


def test_not_found_factory():
	err = FluentRestError.not_found("setup/models/energy")
	assert err.status == 404
	assert "setup/models/energy" in str(err)

6.6 Testing _names_from and _size_from

Because these are now module-level pure functions, they are trivially
importable and testable:

from ansys.fluent.core.rest.client import _names_from, _size_from


@pytest.mark.parametrize("result, expected", [
	(["a", "b", "c"],          ["a", "b", "c"]),
	({"a": {}, "b": {}, "c": {}}, ["a", "b", "c"]),
	(None,                     []),
	(42,                       []),
])
def test_names_from(result, expected):
	assert _names_from(result) == expected


@pytest.mark.parametrize("result, expected", [
	([1, 2, 3],                3),
	({"a": {}, "b": {}},      2),
	({"size": 7, "x": {}},    7),    # explicit size field wins
	(None,                    0),
])
def test_size_from(result, expected):
	assert _size_from(result) == expected

These tests previously required instantiating FluentRestClient to reach the
@staticmethods.


7. Backwards Compatibility

All existing callers continue to work without any modification:

Old usage Status
from ansys.fluent.core.rest import FluentRestClient ✅ unchanged
from ansys.fluent.core.rest import FluentRestError ✅ unchanged
from ansys.fluent.core.rest import connect_to_webserver ✅ re-exported alias
from ansys.fluent.core.rest.client import FluentRestClient ✅ same module
from ansys.fluent.core.rest.client import FluentRestError ❌ moved to errors.py
from ansys.fluent.core.rest.rest_connect import connect_to_webserver ❌ module deleted

The last two rows are internal import paths not present in the public API.
Any code that imported FluentRestError directly from client (rather than
from the package root) will need to update its import to:

from ansys.fluent.core.rest.errors import FluentRestError
# or, equivalently:
from ansys.fluent.core.rest import FluentRestError

8. Dependency Graph

errors.py
	└── (stdlib only: urllib.error)

transport.py
	└── errors.py
	└── (stdlib only: hashlib, json, ssl, time, urllib.request)

client.py
	├── errors.py    (for FluentRestError in delete())
	└── transport.py (for RequestStrategy + HttpRequestStrategy in connect())

__init__.py
	├── client.py
	├── errors.py
	└── transport.py

The dependency graph is acyclic and flows strictly in one direction:
errorstransportclient__init__.

No layer imports from a layer above it. errors.py has no internal
dependencies at all — it only uses the Python standard library. This means
any of the three inner modules can be imported, read, and tested without
pulling in the others.

mayankansys and others added 30 commits March 4, 2026 12:35
Comment thread src/ansys/fluent/core/rest/client.py Outdated

Copilot AI 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.

Pull request overview

Refactors the ansys.fluent.core.rest client into a layered architecture by separating REST error modeling, HTTP transport mechanics, and the Fluent settings API surface, enabling unit-testing of the API layer via an injectable transport strategy.

Changes:

  • Introduces errors.py (FluentRestError) and transport.py (RequestStrategy + HttpRequestStrategy) and rewrites client.py to be transport-agnostic via strategy injection.
  • Updates the package surface in rest/__init__.py (re-exports + connect_to_webserver alias) and adds a real_server pytest marker + real-server fixture scaffolding.
  • Adds a large REST test suite and integration-test fixtures (but the new tests currently target the pre-refactor API and need updating to the new layering).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
tests/test_rest.py Adds REST tests, but currently written against the old FluentRestClient(url, auth_token, ...) API and removed lifecycle/rest_connect module.
tests/conftest.py Adds real-server probing and a real_client fixture for integration tests (needs to use FluentRestClient.connect() under the new design).
src/ansys/fluent/core/rest/transport.py New transport layer with retry/backoff, auth header hashing, and RequestStrategy Protocol.
src/ansys/fluent/core/rest/errors.py New centralized REST error type and transport exception translation.
src/ansys/fluent/core/rest/client.py Rewritten API client using injected strategy; needs to align 404 behavior with documented contract for list/name helpers.
src/ansys/fluent/core/rest/init.py Updates public exports and provides connect_to_webserver alias to FluentRestClient.connect.
pyproject.toml Adds real_server marker.
doc/changelog.d/5220.added.md Adds changelog fragment for the REST layering feature.
doc/changelog.d/5015.added.md Adds changelog fragment related to REST connection.

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

Comment thread tests/test_rest.py Outdated
Comment thread tests/test_rest.py
Comment thread tests/test_rest.py Outdated
Comment thread tests/conftest.py Outdated
Comment thread src/ansys/fluent/core/rest/client.py
Comment thread src/ansys/fluent/core/rest/client.py
Comment thread tests/test_rest.py Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI added 2 commits June 30, 2026 15:35
…rt patches

- Add FakeStrategy test double implementing RequestStrategy.request();
  records all (method, endpoint, body) calls in a queue-based FIFO.
- Fix _client() helper to build FluentRestClient(FakeStrategy())
  instead of the removed FluentRestClient(base_url, auth_token=...) API.
- Add _http_strategy() helper for HttpRequestStrategy-level tests.
- Rename TestFluentRestClientInit → TestHttpRequestStrategyInit;
  assertions now target HttpRequestStrategy attributes (_base_url,
  _headers, _timeout, etc.) and FluentRestClient._api_base.
- Split TestFluentRestClientAuth into TestAuthHelpers (testing the
  _make_auth_headers helper in transport.py) and
  TestHttpRequestStrategyAuth (patching transport.urllib, not client.urllib).
- Rewrite Reads/Writes/NamedObjects/Execute test classes to check
  strategy.calls instead of inspecting urllib Request objects.
- Remove TestFluentRestClientLifecycle (exit/_is_closed/context-manager
  functionality was removed from FluentRestClient in the refactor).
- Rename TestFluentRestClientTransport → TestHttpRequestStrategyTransport;
  all patches now target ansys.fluent.core.rest.transport.urllib.request.urlopen.
- Rewrite TestConnectToWebserver to inspect client._strategy attributes.
- Fix test_all_exports to include HttpRequestStrategy and RequestStrategy.
- Fix classmethod identity checks to use __func__ comparison.
Comment thread src/ansys/fluent/core/rest/client.py Outdated
Comment thread src/ansys/fluent/core/rest/client.py Outdated
Comment thread src/ansys/fluent/core/rest/client.py Outdated
Comment thread src/ansys/fluent/core/rest/client.py Outdated
Comment thread tests/test_rest.py Outdated
Comment thread src/ansys/fluent/core/rest/client.py Outdated
@seanpearsonuk

Copy link
Copy Markdown
Collaborator Author

@Gobot1234 Thanks for the comments. This PR is a refactored version of the current PR for the same version. Not all details are finalised. Tests have not been updated and testing guidance is given in the description. @mayankansys Would you like to either take over this PR or incorporate the concepts into your PR? @Gobot1234's need to be looked at.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Related to dependencies maintenance General maintenance of the repo (libraries, cicd, etc) new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Demonstrate that PyFluent can run transparently over REST or gRPC for solver settings

6 participants