Update to use MediaCreation and general tidy #30
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
foundrytom
force-pushed
the
work/updates
branch
from
5534abf to
b85ae68
Compare
feltech
reviewed
Aug 16, 2023
foundrytom
force-pushed
the
work/updates
branch
5 times, most recently
from
9b670db to
b609db8
Compare
foundrytom
force-pushed
the
work/updates
branch
from
6b35975 to
0016737
Compare
elliotcmorris
approved these changes
Aug 17, 2023
|
Added a pre-emptive commit to drop setting |
elliotcmorris
approved these changes
Aug 22, 2023
Updates the test infrastructure to library relative media, and config relative settings. Pre-configures the resolver. This hopefully simplifies the setup steps, and removes the need for some validation test fixtures. Signed-off-by: Tom Cowland <[email protected]>
Migrates over to media creation, instead of the low-level TraitsData API with hard-coded trait strings. Adds basic decoding of %20 to space, as it is prob quite common. Signed-off-by: Tom Cowland <[email protected]>
When we made this PoC, we noted that in order to support querying extension, mtime, from the manager, as well as to support publishing we'd need access to the entity reference in `_OpenAssetForWrite` and other methods. We opted to pass the ref through `_Resolve` so it would be available to these other methods, and actually do the resolve in `_OpenAsset` and friends. We know performance is important, and we need to ensure that early adopters don't see significantly increased overhead due to OpenAssetIO. Looking again, we can split behaviour, so that the old approach is only done in `_ResolveForNewAsset`. This means that the common case (read) won't end up with numerous redundant resolver per read. Signed-off-by: Tom Cowland <[email protected]>
foundrytom
force-pushed
the
work/updates
branch
from
a305b31 to
029a48f
Compare
We put this in when we were learning about when and where USD invoked the various methods of the API. People have indicated that the overhead of the wrapper to OpenAssetIO would be a key factor during assessment. This commit removes this logging, to avoid any unnecessary overhead. It also makes the code somewhat easier to grok, which should help people using this as a reference implementation. Signed-off-by: Tom Cowland <[email protected]>
Signed-off-by: Tom Cowland <[email protected]>
…ing` The previous implementation doubled up the `isEntityReferenceString` check. Though probably not a big deal, this code will serve as an integration example, and so should illustrate best practice. Tweaks names slightly to help with grokability. Signed-off-by: Tom Cowland <[email protected]>
Returning an empty result seems to be the way to ensure that an error is raised in a meaningful fashion. TF_ERROR doesn't abort, and TF_FATAL_ERROR aborts in a hard fashion in many situations. Signed-off-by: Tom Cowland <[email protected]>
Signed-off-by: Tom Cowland <[email protected]>
This will shortly be removed from OpenAssetIO... Signed-off-by: Tom Cowland <[email protected]>
foundrytom
force-pushed
the
work/updates
branch
from
029a48f to
6f853a4
Compare
feltech
approved these changes
Aug 23, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updates to use MediaCreation, along with general tidying to get this closer to a usable state.
This moves resolve back into resolve, as for the read case, we shouldn't need to bother the manager for the other entry points. As we have a separate write path, then we can do the trick of deferring till later there, without compromising the performance of the higher call-count read path.
I think we have write errors making sense now, but we should still look at ensuring
criticalerrors in Resolve actually cause a real error.TF_FATAL_ERRORseems to be the way to go, but it rather aggressivelySIGABRTs, unless otherwise handled. In Python we don't seem to be able to install aTfDiagnosticsMgrDelegate- resulting in forceful termination - which makes testing rather hard.