Skip to content

[uss_qualifier/resources] Update naming and documentation for ResourceModifyingResource #1474

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.

Sign up for GitHub

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

Open
BenjaminPelletier wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[uss_qualifier/resources] Update naming and documentation for ResourceModifyingResource #1474

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
afc93ca
Update naming and documentation for ResourceModifyingResource
BenjaminPelletier May 20, 2026
8ad1a4d
Fix unit test error and warnings
BenjaminPelletier May 20, 2026
cb3c10b
Add ResourceProvidingResource base class per comment thread
BenjaminPelletier May 21, 2026
2fb5507
Apply @the-glu suggestions
BenjaminPelletier May 26, 2026
c75b95d
Merge remote-tracking branch 'interuss/main' into adjust-resource-mod…
BenjaminPelletier May 26, 2026
44881aa
Add specific error for incorrectly-specified keys per comments
BenjaminPelletier May 27, 2026
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
10 changes: 5 additions & 5 deletions monitoring/uss_qualifier/resources/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ Resources for a given test configuration are all declared in a single global res
3. Every type of test resource must define how to create an instance of the test resource from an instance of the resource specification.


## Resource modifiers
## Resource-modifying resources

A `ResourceModifier` is a resource that wraps another resource and produces variants of it based on an integer index. This is useful when a test scenario needs multiple unique-but-related instances of a resource (e.g., distinct flights derived from a single base flight).
A `ResourceModifyingResource` is a resource capable of spawning other resources by modifying a template/base resource according to a desired key, such as an index. This is useful when a test scenario needs multiple unique-but-related instances of a resource (e.g., distinct flights derived from a single base flight).

To use a `ResourceModifier`:
To use a `ResourceModifyingResource`:

1. Declare it like any other resource, with its `base_resource` dependency pointing to the resource to be modified.
2. When need, call `adjust(index)` to obtain a modified copy of the base resource. Different `index` values produce different (unique) variants; the same `index` produces equivalent results.
2. When a variant of the base/template resource is needed, call `modify(key)` to obtain a modified copy of the base resource. Different `key` values generally produce different variants; the same `key` should produce equivalent results.
Copy link
Copy Markdown
Contributor

@the-glu the-glu May 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Shouldn't it be the opposite ? A different key must produce different variants; the same may produce equivalents result?

I would assume that the spirit is to ensure different resources when needed because that more important that having the same values no?

(We can however have a 'must' for the same key -> same result)

Copy link
Copy Markdown
Member Author

@BenjaminPelletier BenjaminPelletier May 20, 2026

Choose a reason for hiding this comment

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

I'm not sure I understand the question; the change says "different key values -> different variants" + "same key -> equivalent results" -- the same thing as the original, just using "key" instead of "index".

One substantive difference is the removal of "(unique)" because that will probably often be the case, but there's no reason it necessarily needs to be in any case where we want a resource that spawns resources based on a template resource.

Copy link
Copy Markdown
Contributor

@the-glu the-glu May 20, 2026

Choose a reason for hiding this comment

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

That indeed about the "(unique)" removed and the addition of 'generally': since the final goal is to be able to have resources that can be used in parallel without conflicts, I would expect that those resources generator, by 'contract' return different variants in every case, not in most of the cases.

Copy link
Copy Markdown
Member Author

@BenjaminPelletier BenjaminPelletier May 20, 2026

Choose a reason for hiding this comment

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

The use case prompting the creation of this resource needs unique variants in all cases and should always produce equivalent results with equivalent keys. But the way this resource is defined allows it to be used in more general ways. Just because one use of a tool has certain requirements doesn't mean those requirements need to be imposed at the tool level rather than the tool-usage level. For instance, suppose we wanted to iterate over all ordered {uss1, uss2} pairs of participants, but a test needed a token that corresponded with the unordered pair of {uss1, uss2} (so {uss1, uss2} has a different token resource than {uss1, uss3}, but the same token resource as {uss2, uss1}). The suite may still be iterating over ordered pairs, but the resource producing modified versions of the underlying token resource may produce the same modified resource for multiple keys.

Copy link
Copy Markdown
Contributor

@the-glu the-glu May 20, 2026

Choose a reason for hiding this comment

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

Just because one use of a tool has certain requirements doesn't mean those requirements need to be imposed at the tool level rather than the tool-usage level.

Ok, but should we then define an sub-type of the base type than enforce this? I would expect instances used in 'parallel generator' to follow stricter rules (This probably answer the type question bellow as well)

but the resource producing modified versions of the underlying token resource may produce the same modified resource for multiple keys

(True, but in that case since it's unordered, it would be an equal key ({uss1,uss2}=={uss2,uss1}) even if it's has different values. But that don't mean other cases wouldn't exists.)

Copy link
Copy Markdown
Member Author

@BenjaminPelletier BenjaminPelletier May 20, 2026

Choose a reason for hiding this comment

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

Ok, but should we then define an sub-type of the base type than enforce this? I would expect instances used in 'parallel generator' to follow stricter rules (This probably answer the type question bellow as well)

I wouldn't expect we'd need to. A good test design will produce test scenarios that don't interfere with each other. Not interfering sometimes means "unique" modified resources (a necessary but not sufficient condition to deconflict geospatial resources), but not always (like in the case of iterating over ordered pairs but only needing unique unordered pairs for the token resource). But if we did need "unique" modified resources, I'd expect that to be introduced along with the thing that actually needs that constraint. The content in this PR (and #1465) doesn't need uniqueness -- the concept of stamping out modified copies of a template resource based on a key is a good standalone concept that doesn't need the additional constraint to work.

(True, but in that case since it's unordered, it would be an equal key ({uss1,uss2}=={uss2,uss1}) even if it's has different values. But that don't mean other cases wouldn't exists.)

No, the key would be ordered as the test scenarios (or whatever is being primarily iterated) are distinguishable based on order in the example, but the particular resource being generated by modifying a base resource doesn't depend on order. To elaborate on the example: suppose we have a test scenario that has two roles and uses a token resource that needs to be "unique" according to participants in the scenario, but not according to what roles they're playing. The action generator would iterate over every (role1, role2) assignment of the {uss1, uss2, uss3, ...} participants when instantiating test scenarios. And, each test scenario would need a token resource based on some template token resource. But, the token resource needed would only depend on the participants rather than role assignments, so the modified token resources spawned would not be unique. At the same time, geospatial deconfliction is needed, so every set of flight intents would need to be "unique".

Scenario Role 1 Role 2 Flight intents resource Token resource
1 uss1 uss1 A X
2 uss1 uss2 B Y
3 uss2 uss1 C Y
4 uss2 uss2 D Z

Copy link
Copy Markdown
Contributor

@the-glu the-glu May 21, 2026

Choose a reason for hiding this comment

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

A good test design will produce test scenarios that don't interfere with each other.

Ok - that move the "responsibility" up to test definitions to only combine things that works together without an explicit 'contract', but that valid.


The base resource itself remains available as `base_resource` on the modifier.
The base/template resource itself remains available as `base_resource` on the modifier.
6 changes: 4 additions & 2 deletions monitoring/uss_qualifier/resources/dev/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from .noop import NoOpResource as NoOpResource
from .test_exclusions import TestExclusionsResource as TestExclusionsResource
from .test_modifier import TestModifierModifierResource as TestModifierModifierResource
from .test_modifier import TestModifierResource as TestModifierResource
from .test_modifier import (
NumberGeneratorModifierResource as NumberGeneratorModifierResource,
)
from .test_modifier import NumberGeneratorResource as NumberGeneratorResource
53 changes: 38 additions & 15 deletions monitoring/uss_qualifier/resources/dev/test_modifier.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
from implicitdict import ImplicitDict

from monitoring.uss_qualifier.resources.resource import Resource, ResourceModifier
from monitoring.uss_qualifier.resources.resource import (
Resource,
ResourceProvidingResource,
)


class TestModifierSpecification(ImplicitDict):
class NumberGeneratorSpecification(ImplicitDict):
base_id: int


class TestModifierResource(Resource[TestModifierSpecification]):
"""TestModifierResource is a simple resource returing 10 number, starting from base_id. Used for unit tests."""
class NumberGeneratorResource(Resource[NumberGeneratorSpecification]):
"""A simple resource returing 10 numbers, starting from base_id. Used for unit tests."""

_spec: TestModifierSpecification
_spec: NumberGeneratorSpecification

def __init__(
self,
specification: TestModifierSpecification,
specification: NumberGeneratorSpecification,
resource_origin: str,
):
super().__init__(specification, resource_origin)
Expand All @@ -24,22 +27,42 @@ def build_ids(self) -> list[int]:
return list(range(self._spec.base_id, self._spec.base_id + 10))


class TestModifierModifierSpecification(ImplicitDict):
class NumberGeneratorModifierSpecification(ImplicitDict):
shift_interval: int


class TestModifierModifierResource(
ResourceModifier[TestModifierModifierSpecification, TestModifierResource]
class NumberGeneratorModifierResource(
ResourceProvidingResource[
NumberGeneratorModifierSpecification, NumberGeneratorResource
]
):
"""Modifier for a TestModifierResource. Used for unit tests."""
"""Modifier for a NumberGeneratorResource. Used for unit tests."""

def adjust(self, index: int) -> TestModifierResource:
_spec: NumberGeneratorModifierSpecification
base_resource: NumberGeneratorResource

# 'Clone' the resource with new specs
return TestModifierResource(
TestModifierSpecification(
def __init__(
self,
specification: NumberGeneratorModifierSpecification,
resource_origin: str,
base_resource: NumberGeneratorResource,
):
super().__init__(specification, resource_origin)
self._spec = specification
self.base_resource = base_resource

def _modified_resource_origin(self, index: int) -> str:
Copy link
Copy Markdown
Contributor

@the-glu the-glu May 27, 2026

Choose a reason for hiding this comment

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

This one need to be updated with new signature

Copy link
Copy Markdown
Member Author

@BenjaminPelletier BenjaminPelletier May 27, 2026

Choose a reason for hiding this comment

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

This function returns a resource origin string for the specified index. It isn't part of the ResourceProvidingResource abstraction. Why would it have a different signature?

Copy link
Copy Markdown
Contributor

@the-glu the-glu May 27, 2026

Choose a reason for hiding this comment

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

Ho ok, I confused it with _provided_resource_origin of the base class (since modified where changed from provided)

return f"Modification {index} of {self.base_resource.resource_origin} by {self.resource_origin}"

def provide_resource_for(self, **kwargs) -> NumberGeneratorResource:
index = kwargs["index"]
assert isinstance(index, int)

# 'Clone' the base resource with new specs
return NumberGeneratorResource(
NumberGeneratorSpecification(
base_id=self.base_resource._spec.base_id
+ self._spec.shift_interval * index,
),
resource_origin=self.base_resource.resource_origin,
resource_origin=self._modified_resource_origin(index),
)
38 changes: 12 additions & 26 deletions monitoring/uss_qualifier/resources/resource.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,34 +43,20 @@ def is_type(self, resource_type: str) -> bool:
ResourceType = TypeVar("ResourceType", bound=Resource)


class ResourceModifier[SpecificationType: ImplicitDict, ResourceType](
Resource[SpecificationType], ABC
):
"""A specifc type of resources that can return adjusted an resource that shall unique based on a specifc 'index'.
The underlying resource shall be a dependency named 'base_resource'.

Concrete subclass must implement 'adjust' as needed.
"""

_spec: SpecificationType
base_resource: ResourceType

def __init__(
self,
specification: SpecificationType,
resource_origin: str,
base_resource: ResourceType,
):
super().__init__(specification, resource_origin)
self._spec = specification
self.base_resource = base_resource
class ResourceProvidingResource[
SpecificationType: ImplicitDict,
ResourceType: Resource,
](Resource[SpecificationType], ABC):
"""Resource capable of spawning ResourceType resources according to a desired key, such as an index."""

@abstractmethod
def adjust(self, index: int) -> ResourceType:
"""
Return a new instance of the base resource, modified to be unique based on 'index' value.
"""
pass
def provide_resource_for(self, **kwargs) -> ResourceType:
"""Provide a resource corresponding with the provided key(s) (e.g., index, USS pair, etc)."""
raise NotImplementedError()

def _provided_resource_origin(self, key_name: str) -> str:
Copy link
Copy Markdown
Contributor

@the-glu the-glu May 27, 2026

Choose a reason for hiding this comment

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

What the keyname there? Shouldn't it be the same as provide_resource_for?

Copy link
Copy Markdown
Member Author

@BenjaminPelletier BenjaminPelletier May 27, 2026

Choose a reason for hiding this comment

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

It's the name of the key -- the thing that determined the generated resource (index, USS pair, etc). A ResourceProvidingResource can accept potentially many keyword arguments, but it needs to select one of them to actually use. And, a key doesn't necessarily have a native string representation (e.g., a USS pair). So, the implementation needs to provide a string representation to get a resource origin string. We changed provide_resource_for to simply accept **kwargs -- that is not usable directly for anything in general because each the specific implementation needs to extract and translate whatever type(s) of key(s) it accepts. This is one downside to using **kwargs in the abstract class.

"""Method that should generally be used to describe the origin of a resource provided by this resource."""
return f"Resource for {key_name} provided by {self.resource_origin}"


class MissingResourceError(ValueError):
Expand Down
70 changes: 36 additions & 34 deletions monitoring/uss_qualifier/resources/resources_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,72 +5,74 @@
ResourceID,
)
from monitoring.uss_qualifier.resources.dev.test_modifier import (
TestModifierModifierSpecification,
TestModifierSpecification,
NumberGeneratorModifierSpecification,
NumberGeneratorSpecification,
)
from monitoring.uss_qualifier.resources.resource import create_resources


class TestResourceModifier(unittest.TestCase):
def _build_test_modifier_declaration(
class TestModifierResource(unittest.TestCase):
def _build_number_generator_declaration(
self, base_id
) -> dict[ResourceID, ResourceDeclaration]:
return {
"test": ResourceDeclaration(
resource_type="resources.dev.TestModifierResource",
specification=TestModifierSpecification(base_id=base_id),
"number_generator": ResourceDeclaration(
resource_type="resources.dev.NumberGeneratorResource",
specification=NumberGeneratorSpecification(base_id=base_id),
)
}

def _build_test_modifier_modifier_declaration(
def _build_modifier_declaration(
self, base_id, shift_interval
) -> dict[ResourceID, ResourceDeclaration]:
return {
"test": self._build_test_modifier_declaration(base_id)["test"],
"test_modifier": ResourceDeclaration(
resource_type="resources.dev.TestModifierModifierResource",
specification=TestModifierModifierSpecification(
"number_generator": self._build_number_generator_declaration(base_id)[
"number_generator"
],
"modifier": ResourceDeclaration(
resource_type="resources.dev.NumberGeneratorModifierResource",
specification=NumberGeneratorModifierSpecification(
shift_interval=shift_interval
),
dependencies={
"base_resource": "test",
"base_resource": "number_generator",
},
),
}

def test_base_resource(self):
"""Test basic usage of the resource"""
declaration = self._build_test_modifier_declaration(42)
declaration = self._build_number_generator_declaration(42)

resources = create_resources(declaration, "test", True)
assert "test" in resources
resources = create_resources(declaration, "unittest", True)
assert "number_generator" in resources

resource = resources["test"]
resource = resources["number_generator"]

assert resource.build_ids() == [42, 43, 44, 45, 46, 47, 48, 49, 50, 51]

def test_base_resource_base_id(self):
"""Test that base id works as expected"""

declaration = self._build_test_modifier_declaration(52)
declaration = self._build_number_generator_declaration(52)

resources = create_resources(declaration, "test", True)
assert "test" in resources
resources = create_resources(declaration, "unittest", True)
assert "number_generator" in resources

resource = resources["test"]
resource = resources["number_generator"]

assert resource.build_ids() == [52, 53, 54, 55, 56, 57, 58, 59, 60, 61]

def test_modifier_resource(self):
"""Test basic usage of the resource modifier"""
declaration = self._build_test_modifier_modifier_declaration(42, 10)
"""Test basic usage of the resource modifier resource"""
declaration = self._build_modifier_declaration(42, 10)

resources = create_resources(declaration, "test", True)
assert "test_modifier" in resources
resources = create_resources(declaration, "unittest", True)
assert "modifier" in resources

resource = resources["test_modifier"]
resource = resources["modifier"]

assert resource.adjust(0).build_ids() == [
assert resource.provide_resource_for(index=0).build_ids() == [
42,
43,
44,
Expand All @@ -82,7 +84,7 @@ def test_modifier_resource(self):
50,
51,
]
assert resource.adjust(1).build_ids() == [
assert resource.provide_resource_for(index=1).build_ids() == [
52,
53,
54,
Expand All @@ -97,14 +99,14 @@ def test_modifier_resource(self):

def test_modifier_resource_shift(self):
"""Test shift usage of the resource modifier"""
declaration = self._build_test_modifier_modifier_declaration(42, 20)
declaration = self._build_modifier_declaration(42, 20)

resources = create_resources(declaration, "test", True)
assert "test_modifier" in resources
resources = create_resources(declaration, "unittest", True)
assert "modifier" in resources

resource = resources["test_modifier"]
resource = resources["modifier"]

assert resource.adjust(0).build_ids() == [
assert resource.provide_resource_for(index=0).build_ids() == [
42,
43,
44,
Expand All @@ -116,7 +118,7 @@ def test_modifier_resource_shift(self):
50,
51,
]
assert resource.adjust(1).build_ids() == [
assert resource.provide_resource_for(index=1).build_ids() == [
Copy link
Copy Markdown
Contributor

@the-glu the-glu May 27, 2026

Choose a reason for hiding this comment

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

Nit: Shouldn't we also test the case with unknown key generating an error?

Making me thinking: shouldn't we have a predetermined error like UnprocessableKeys(ValueError) to have a predetermined error?

Copy link
Copy Markdown
Member Author

@BenjaminPelletier BenjaminPelletier May 27, 2026

Choose a reason for hiding this comment

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

Added (both)

62,
63,
64,
Expand Down
4 changes: 2 additions & 2 deletions ...er/TestModifierModifierSpecification.json → ...NumberGeneratorModifierSpecification.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"$id": "https://github.com/interuss/monitoring/blob/main/schemas/monitoring/uss_qualifier/resources/dev/test_modifier/TestModifierModifierSpecification.json",
"$id": "https://github.com/interuss/monitoring/blob/main/schemas/monitoring/uss_qualifier/resources/dev/test_modifier/NumberGeneratorModifierSpecification.json",
"$schema": "https://json-schema.org/draft/2020-12/schema",
"description": "monitoring.uss_qualifier.resources.dev.test_modifier.TestModifierModifierSpecification, as defined in monitoring/uss_qualifier/resources/dev/test_modifier.py",
"description": "monitoring.uss_qualifier.resources.dev.test_modifier.NumberGeneratorModifierSpecification, as defined in monitoring/uss_qualifier/resources/dev/test_modifier.py",
"properties": {
"$ref": {
"description": "Path to content that replaces the $ref",
Expand Down
4 changes: 2 additions & 2 deletions ...t_modifier/TestModifierSpecification.json → ...odifier/NumberGeneratorSpecification.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"$id": "https://github.com/interuss/monitoring/blob/main/schemas/monitoring/uss_qualifier/resources/dev/test_modifier/TestModifierSpecification.json",
"$id": "https://github.com/interuss/monitoring/blob/main/schemas/monitoring/uss_qualifier/resources/dev/test_modifier/NumberGeneratorSpecification.json",
"$schema": "https://json-schema.org/draft/2020-12/schema",
"description": "monitoring.uss_qualifier.resources.dev.test_modifier.TestModifierSpecification, as defined in monitoring/uss_qualifier/resources/dev/test_modifier.py",
"description": "monitoring.uss_qualifier.resources.dev.test_modifier.NumberGeneratorSpecification, as defined in monitoring/uss_qualifier/resources/dev/test_modifier.py",
"properties": {
"$ref": {
"description": "Path to content that replaces the $ref",
Expand Down
Loading