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

Make syncType less prone to caching invalid values #187

Closed
nick4598 opened this issue Jun 17, 2024 · 2 comments · Fixed by #190
Closed

Make syncType less prone to caching invalid values #187

nick4598 opened this issue Jun 17, 2024 · 2 comments · Fixed by #190

Comments

@nick4598
Copy link
Collaborator

nick4598 commented Jun 17, 2024

Writing this workup item after a call with transformation services team

Problem

Calling getters on the transformer such as :

  • provenanceDb
  • provenanceSourceDb
  • isForwardSync
  • isReverseSync

Would lock in incorrect values if they were called too early. This is because a property 'isSynchronization' needs to be set to true before the syncDirection is accurately determined.

We had a few potential solutions:

Solution 1

use a private property to make sure that processAll / processChanges has been called before accessing provenanceDb / provenanceSourceDb getters. Throw error if not.
Problems:

  • services team could easily set this private property if they wanetd.

Solution 2

Add hooks / callbacks that would be called on events such as onTransformerInitialized.

  • prov db and sync getters would be removed, but that information would be passed ot the callbacks.

Problems:

  • callbacks make consumers dependent on the order they're called in

Solution 3

Pass to transformer ctor additional options:
iSsynchronization: boolean. processAll and processChanges will be merged into one process() method
initialize would be moved into function which is responsibility of consumers to call. Then it shoudl be safe to access provenanceDb/sourceDb/ syncDirection.

Open Questions

  • Does transformation services ever set isSynchronization to true during a processAll?
  • Is there such thing as a processAll reverseSync? If isSynchronization is false, which it is in processAll. then that defauslt to assuming a forward sync.
  • If consumer fails to call initialize, should we call it for them in the process function?
@nick4598
Copy link
Collaborator Author

Solution 3 from vilius seems the best, we should proceed with that.

@nick4598
Copy link
Collaborator Author

Answers to the open Questions;

Does transformation services ever set isSynchronization to true during a processAll?

  • No. We only do that when we are going to call processChanges and want to call initialize before that.

Is there such thing as a processAll reverseSync? If isSynchronization is false, which it is in processAll. then that defauslt to assuming a forward sync.

  • Reverse synchronization is always a change processing workflow on our side.

If consumer fails to call initialize, should we call it for them in the process function?

  • From our perspective, I would rather have it manually invoked only and have the transformer throw something like a "NotInitializedError" if we forgot to do it. Reason for that being is that initialize will be a complex and heavy operation, and the transformer might even add elements into iModels when you call it. I wouldn't want such a thing be implicitly called, and the user should understand it and call it explicitly. Similar to "I agree with terms and conditions" Accept button

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant