Refactor `getRelatedReferences` #847

foundrytom · 2023-03-23T10:21:48Z

What

The current 'three methods in one' should be refactored in to individual methods.

Why

It has been noted that 'remembering all the rules for the lengths of the input arrays' is confusing and fragile.

foundrytom · 2023-04-13T13:09:38Z

Proposal:

entitiesWithRelationship, entitesWithRelationships

Then for #16: add* and remove*

elliotcmorris · 2023-04-13T13:31:04Z

Do we mean entitiesWithRelationship and entityWithRelationships, ie a many->one and a one->many?

foundrytom · 2023-04-13T15:49:50Z

Capturing offline discussion:

Naming things is hard.
Were struggling to find a clear winner.
Were prob going to leave add/remove untill later.
Finding a less-than-perfect solution that works for all potential permutations is more valuable than perfection that constrains options later.

Proposal:

Sleep on it, and if we can't come up with a better option by e.o.d 14th the just go with this:

entitiesWithRelationship(entityRefs, relationship, ... )
entitiesWithRelationsips(entityRef, relationships, ... )

addRelationship(targetRefs, relationship, listOfRefLists, ... )
addRelationships(targetRef, relationships, listOReffLists, ... )

removeRelationship(targetRefs, relationship, listOfRefLists, ... )
removeRelationships(targetRef, relationships, listOfRefLists, ... )

foundrytom · 2023-05-03T15:36:09Z

Subsequent to some further discussion/casual surveying have renamed the query methods to better align with their counterparts and other API methods:

getWithRelationship(entityRefs, relationship, ... )
getWithRelationships(entityRef, relationships, ... )

The old "two lists" formulation was hard to grok, and difficult to implement effectively. Splitting into two discrete methods better outlines the two potential batch axis. This new approach makes it clear the two well-known divergent use cases and allows for a simpler code path in each - without the need to interrogate the input arrays to work out which pattern is being used (one-to-many or many-to-one). Naming this was hard, after assorted rounds, this option emerged as a 'best of a bad bunch', as it is at least consitent with future 'addRelationship' and 'removeRelationship' methods, along with our more general noun/verb conventions. There is certainly ambiguity as the singular/plural difference is subtle, but it's the best we could come up with. Closes OpenAssetIO#847 Signed-off-by: Tom Cowland <tom@foundry.com>

We have always documented that supporting relationship queries is optional for a manager (though highly recommended). Despite this the API methods were marked as being abstract. We only got away with this as since the move to Python 3, ABC enforcement wasn't working. This adds default implementations of `getRelationship` and `getRelationships` that simply returns an empty list for each element of the batch. Part of OpenAssetIO#847 Signed-off-by: Tom Cowland <tom@foundry.com>

The old "two lists" formulation was hard to grok, and difficult to implement effectively. Splitting into two discrete methods better outlines the two potential batch axis. This new approach makes it clear the two well-known divergent use cases and allows for a simpler code path in each - without the need to interrogate the input arrays to work out which pattern is being used (one-to-many or many-to-one). Naming this was hard, after assorted rounds, this option emerged as a 'best of a bad bunch', as it is at least consitent with future 'addRelationship' and 'removeRelationship' methods, along with our more general noun/verb conventions. There is certainly ambiguity as the singular/plural difference is subtle, but it's the best we could come up with. Closes OpenAssetIO#847 Signed-off-by: Tom Cowland <tom@foundry.com>

We have always documented that supporting relationship queries is optional for a manager (though highly recommended). Despite this the API methods were marked as being abstract. We only got away with this as since the move to Python 3, ABC enforcement wasn't working. This adds default implementations of `getRelationship` and `getRelationships` that simply returns an empty list for each element of the batch. Part of OpenAssetIO#847 Signed-off-by: Tom Cowland <tom@foundry.com>

The old "two lists" formulation was hard to grok, and difficult to implement effectively. Splitting into two discrete methods better outlines the two potential batch axis. This new approach makes it clear the two well-known divergent use cases and allows for a simpler code path in each - without the need to interrogate the input arrays to work out which pattern is being used (one-to-many or many-to-one). Naming this was hard, after assorted rounds, this option emerged as a 'best of a bad bunch', as it is at least consitent with future 'addRelationship' and 'removeRelationship' methods, along with our more general noun/verb conventions. There is certainly ambiguity as the singular/plural difference is subtle, but it's the best we could come up with. Closes OpenAssetIO#847 Signed-off-by: Tom Cowland <tom@foundry.com>

We have always documented that supporting relationship queries is optional for a manager (though highly recommended). Despite this the API methods were marked as being abstract. We only got away with this as since the move to Python 3, ABC enforcement wasn't working. This adds default implementations of `getRelationship` and `getRelationships` that simply returns an empty list for each element of the batch. Part of OpenAssetIO#847 Signed-off-by: Tom Cowland <tom@foundry.com>

The old "two lists" formulation was hard to grok, and difficult to implement effectively. Splitting into two discrete methods better outlines the two potential batch axis. This new approach makes it clear the two well-known divergent use cases and allows for a simpler code path in each - without the need to interrogate the input arrays to work out which pattern is being used (one-to-many or many-to-one). Naming this was hard, after assorted rounds, this option emerged as a 'best of a bad bunch', as it is at least consitent with future 'addRelationship' and 'removeRelationship' methods, along with our more general noun/verb conventions. There is certainly ambiguity as the singular/plural difference is subtle, but it's the best we could come up with. Closes OpenAssetIO#847 Signed-off-by: Tom Cowland <tom@foundry.com>

We have always documented that supporting relationship queries is optional for a manager (though highly recommended). Despite this the API methods were marked as being abstract. We only got away with this as since the move to Python 3, ABC enforcement wasn't working. This adds default implementations of `getRelationship` and `getRelationships` that simply returns an empty list for each element of the batch. Part of OpenAssetIO#847 Signed-off-by: Tom Cowland <tom@foundry.com>

The old "two lists" formulation was hard to grok, and difficult to implement effectively. Splitting into two discrete methods better outlines the two potential batch axis. This new approach makes it clear the two well-known divergent use cases and allows for a simpler code path in each - without the need to interrogate the input arrays to work out which pattern is being used (one-to-many or many-to-one). Naming this was hard, after assorted rounds, this option emerged as a 'best of a bad bunch', as it is at least consitent with future 'addRelationship' and 'removeRelationship' methods, along with our more general noun/verb conventions. There is certainly ambiguity as the singular/plural difference is subtle, but it's the best we could come up with. Closes OpenAssetIO#847 Signed-off-by: Tom Cowland <tom@foundry.com>

We have always documented that supporting relationship queries is optional for a manager (though highly recommended). Despite this the API methods were marked as being abstract. We only got away with this as since the move to Python 3, ABC enforcement wasn't working. This adds default implementations of `getWithRelationship` and `getWithRelationships` that simply returns an empty list for each element of the batch. Part of OpenAssetIO#847 Signed-off-by: Tom Cowland <tom@foundry.com> Signed-off-by: Tom Cowland <tom@foundry.com>

The old "two lists" formulation was hard to grok, and difficult to implement effectively. Splitting into two discrete methods better outlines the two potential batch axis. This new approach makes it clear the two well-known divergent use cases and allows for a simpler code path in each - without the need to interrogate the input arrays to work out which pattern is being used (one-to-many or many-to-one). Naming this was hard, after assorted rounds, this option emerged as a 'best of a bad bunch', as it is at least consitent with future 'addRelationship' and 'removeRelationship' methods, along with our more general noun/verb conventions. There is certainly ambiguity as the singular/plural difference is subtle, but it's the best we could come up with. Closes OpenAssetIO#847 Signed-off-by: Tom Cowland <tom@foundry.com>

We have always documented that supporting relationship queries is optional for a manager (though highly recommended). Despite this the API methods were marked as being abstract. We only got away with this as since the move to Python 3, ABC enforcement wasn't working. This adds default implementations of `getWithRelationship` and `getWithRelationships` that simply returns an empty list for each element of the batch. Part of OpenAssetIO#847 Signed-off-by: Tom Cowland <tom@foundry.com> Signed-off-by: Tom Cowland <tom@foundry.com>

The old "two lists" formulation was hard to grok, and difficult to implement effectively. Splitting into two discrete methods better outlines the two potential batch axis. This new approach makes it clear the two well-known divergent use cases and allows for a simpler code path in each - without the need to interrogate the input arrays to work out which pattern is being used (one-to-many or many-to-one). Naming this was hard, after assorted rounds, this option emerged as a 'best of a bad bunch', as it is at least consitent with future 'addRelationship' and 'removeRelationship' methods, along with our more general noun/verb conventions. There is certainly ambiguity as the singular/plural difference is subtle, but it's the best we could come up with. Closes OpenAssetIO#847 Signed-off-by: Tom Cowland <tom@foundry.com>

We have always documented that supporting relationship queries is optional for a manager (though highly recommended). Despite this the API methods were marked as being abstract. We only got away with this as since the move to Python 3, ABC enforcement wasn't working. This adds default implementations of `getWithRelationship` and `getWithRelationships` that simply returns an empty list for each element of the batch. Part of OpenAssetIO#847 Signed-off-by: Tom Cowland <tom@foundry.com> Signed-off-by: Tom Cowland <tom@foundry.com>

The old "two lists" formulation was hard to grok, and difficult to implement effectively. Splitting into two discrete methods better outlines the two potential batch axis. This new approach makes it clear the two well-known divergent use cases and allows for a simpler code path in each - without the need to interrogate the input arrays to work out which pattern is being used (one-to-many or many-to-one). Naming this was hard, after assorted rounds, this option emerged as a 'best of a bad bunch', as it is at least consitent with future 'addRelationship' and 'removeRelationship' methods, along with our more general noun/verb conventions. There is certainly ambiguity as the singular/plural difference is subtle, but it's the best we could come up with. Closes OpenAssetIO#847 Signed-off-by: Tom Cowland <tom@foundry.com>

We have always documented that supporting relationship queries is optional for a manager (though highly recommended). Despite this the API methods were marked as being abstract. We only got away with this as since the move to Python 3, ABC enforcement wasn't working. This adds default implementations of `getWithRelationship` and `getWithRelationships` that simply returns an empty list for each element of the batch. Part of OpenAssetIO#847 Signed-off-by: Tom Cowland <tom@foundry.com> Signed-off-by: Tom Cowland <tom@foundry.com>

The old "two lists" formulation was hard to grok, and difficult to implement effectively. Splitting into two discrete methods better outlines the two potential batch axis. This new approach makes it clear the two well-known divergent use cases and allows for a simpler code path in each - without the need to interrogate the input arrays to work out which pattern is being used (one-to-many or many-to-one). Naming this was hard, after assorted rounds, this option emerged as a 'best of a bad bunch', as it is at least consistent with future 'addRelationship' and 'removeRelationship' methods, along with our more general noun/verb conventions. There is certainly ambiguity as the singular/plural difference is subtle, but it's the best we could come up with. These new methods also use the batch callback pattern established with the other batch API signatures. In addition, we have always documented that supporting relationship queries is optional for a manager (though highly recommended). Despite this the API methods were marked as being abstract. We only got away with this as since the move to Python 3, ABC enforcement wasn't working. There are now default implementations of `getWithRelationship` and `getWithRelationships` that simply return an empty list for each element of the batch. NB. This commit disables BAL integration tests due to the API break. Closes #847 Closes #919 Signed-off-by: Tom Cowland <tom@foundry.com>

Until a13 is released, we need to build our own OpenAssetIO from src due to API changes. See OpenAssetIO/OpenAssetIO#847 Signed-off-by: Tom Cowland <tom@foundry.com>

foundrytom added the api change label Mar 23, 2023

foundrytom assigned foundrytom and unassigned foundrytom Apr 4, 2023

foundrytom added this to the Core stable milestone Apr 13, 2023

foundrytom mentioned this issue Apr 17, 2023

[Epic] C++ API migration #897

Open

foundrytom self-assigned this May 3, 2023

foundrytom mentioned this issue May 3, 2023

Update implementation of getRelatedReferences to getWithRelationship[s] OpenAssetIO/OpenAssetIO-Manager-BAL#34

Closed

foundrytom mentioned this issue May 4, 2023

Refactor getRelatedReferences #915

Merged

foundrytom mentioned this issue May 9, 2023

Migrate relationship methods to callbacks #919

Closed

foundrytom added the feature branch For issues that should target a feature branch label May 9, 2023

foundrytom closed this as completed May 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor `getRelatedReferences` #847

Refactor `getRelatedReferences` #847

foundrytom commented Mar 23, 2023

foundrytom commented Apr 13, 2023

elliotcmorris commented Apr 13, 2023

foundrytom commented Apr 13, 2023 •

edited

foundrytom commented May 3, 2023 •

edited

Refactor getRelatedReferences #847

Refactor getRelatedReferences #847

Comments

foundrytom commented Mar 23, 2023

What

Why

foundrytom commented Apr 13, 2023

elliotcmorris commented Apr 13, 2023

foundrytom commented Apr 13, 2023 • edited

foundrytom commented May 3, 2023 • edited

Refactor `getRelatedReferences` #847

Refactor `getRelatedReferences` #847

foundrytom commented Apr 13, 2023 •

edited

foundrytom commented May 3, 2023 •

edited