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

Proposal of architecture for the SDK #22

Closed
l0r1s opened this issue May 12, 2023 · 9 comments
Closed

Proposal of architecture for the SDK #22

l0r1s opened this issue May 12, 2023 · 9 comments
Assignees

Comments

@l0r1s
Copy link
Contributor

l0r1s commented May 12, 2023

This schema is a proposal of an architecture that could be done for the SDK:

  • It favors isolation of components which make testing of units easier
  • Definition of abstract/concrete boundaries between crates depending on needs
  • Use of features when not all dependencies of the given crate needed
  • Easily extendable

sdk_updated

Open for your suggestions !

@l0r1s l0r1s self-assigned this May 12, 2023
@pepoviola
Copy link
Collaborator

Looks good, thanks @l0r1s . One thing we can start design is how will be the api between them. I think for example that the configuration crate should produce a NetworkConfiguration (in json format) that will be used by the orchestrator as input to deploy the network, and the orchestrator must produce a running Network (in json format) that will be used by the test-runner to run the test.
This way we can swap crates/modules if they supply the expected json schema as input.

Thanks!!

@l0r1s
Copy link
Contributor Author

l0r1s commented May 12, 2023

Thanks you for your feedback @pepoviola

I'm trying to understand why we would like to use JSON to communicate between crate because I think that if we use JSON instead of Rust types between the crates, we lose the purpose of having type safe code in Rust, given the module can be loosely coupled.

What we could do is having the orchestrator depends on a generic or associated type for the config, which make it modular and possibly extendable by the end user (having a default associated type or generic set to the config we use).

For the part of the test-runner crate, in my experience, having a concrete dependency (not a trait), is not a problem and it can even depends on the generic config (extended or not), to handle eventual modification of the orchestrator by the end user, we could do a system of hooks/plugins/middlewares à la tower that are registered on the orchestrator at different points, depending of need but by keeping the flexibility, and re-exported by the test-runner crate, which has concrete dependency on the orchestrator, for example:

impl<C: Config> Hooks for Orchestrator {
    fn preParachainBootnodeStart(hook: Box<Fn (C) -> C>);
}

or even something taking the network:

impl<C: Config, N: Network, E: Error> Hooks for Orchestrator {
    fn preParachainBootnodeStart(hook: Box<Fn (C, N) -> Result<(C, N), E>)>);
}

(by the way, this code is simplified but the type could be wrapped in a struct with unnamed fields)

That way we keep the boundary loose and the code statically typed, still having the possibility to dump the network configuration from the configuration crate if needed (and parse it back).

@pepoviola
Copy link
Collaborator

Yes, I'm agree. I was thinking more in how we can allow to integrate with others system (outside rust) but now you mention a simple serialization/deserialization will be cover our needs. I like your approach and I think we should start working on this direction.

Thanks!

@wirednkod
Copy link
Contributor

I would like to go through more on the dependencies mentioned in the schema as they look strange to me;
Serialization/deserialization maybe its not the way to go as we should stick to type safe and associated typings; We could though have a validation service for the JSONs that are communicated through the APIs

Also some extra parts that I think need more elaboration on the schema:
a) the "depends-on concrete/abstract" parts;
b) the logic of existence for support crate and why test runner, orchestrator and provider crates depends on that;
c) Why there is configuration structrures in both orchestrator and configuration crates?
d) what is the "shared" part on each of the proposed crates?

@l0r1s
Copy link
Contributor Author

l0r1s commented May 16, 2023

Thanks you for your feedback @wirednkod 👍

In my opinion, serialization/deserialization of the configuration can be usefull if we want to "dump" or "load" the configuration into local files that can be shared or used for debugging. The validation should be presents when you load/deserialize an object into a structure, which helps to hold what we call invariants, basically, these are rules that always hold true for the lifetime of an object, this is not specific to Rust but for example, if you have a Configuration object and you do validation when you create the object (using builders or deserialization) and all your function/method take this object, you are sure that the object is valid.

Following with your questions:

a) The depends-on concrete/abstract parts expose the different types of dependencies, a concrete dependency is a dependency on the concrete type (struct in that case) and not an abstraction (traits in that case).

To give some examples, from what I understand, we don't want the test-runner to be used without the orchestrator, so we can safely say that the orchestrator is a concrete dependency to the test-runner, it doesn't needs to be an abstraction. Based on the fact that the orchestrator doesn't have concrete dependencies himself, he has abstract dependencies on providers (which can be faked for testing) and same for support crate (filesystem/network,...) which makes it easily testable.

On the other side, the orchestrator can depends on the support crate but we don't want to assume which kind of filesystem we are dealing with or which network utilities are present on the system, same goes for logging, we don't know if the user want to log to a file or to the terminal, so we abstract them which makes it easy to have multiple implementations and test it easily by faking them during tests.

b) The existence of the support crate is to abstract away things that doesn't matters in our use case, they support our code but they are irrelevant (we can log to terminal/file/cloud and we can write to filesystem/cloud, or in memory for tests). The abstractions are here so you can still test and work on your code perfectly but these are not the main focus.

c) The crates paths are a mistake, thanks you for seeing that, I will correct them 🎯

d) The shared part of each crate include types/abstractions that does not belongs to the support crate and are only used internally to the crate.

I hope makes some parts more clear, don't hesitate if you have any other questions !

@wirednkod
Copy link
Contributor

Thank you @l0r1s
I think we should keep this issue open as the main Architecture approach since we all agreed that is a very good basic to work on

@pepoviola
Copy link
Collaborator

I think we can convert into a discussion, wdyt?

@l0r1s
Copy link
Contributor Author

l0r1s commented May 29, 2023

Absolutely

@pepoviola
Copy link
Collaborator

Closing here, we can re-open or moving to discussion if needed.

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

No branches or pull requests

3 participants