system services feature: solve import cycle between systemservices & api pkgs

[ilgooz]

No description provided.

[krhubert]

There is still the import cycle, you've just moved the deploy function to deployer package, but the deploy is still done by API.

To fix this issue:

• create deploy package with following funcs/methods: deploy.FromPath, deploy.FromGit, -deploy.FromGzippedTar
• use those funcs/methods inside sytemservices
• use those func/methods inside api
• repeat the same for service start* functions

In such approach, systemservices won't be importing api package.

[ilgooz]

@krhubert There is no import cycle anymore, can you show me the import lines from cycling packages?

The deploy pkg you're proposing is similar to pkg that I proposed on from but with the name native.
You answered as "how many layers API of should be there?" and we went with the mapping idea of yours.

It's late to do this change now, we can discuss about it later and maybe re-consider changing the architecture after we merge waiting PRs that depends on system services.

[antho1404]

this one is including a package that includes the api so the import cycle will still be here.

the systemservices package should not have any knowledge of the deployer but the deployer should have knowledge of the systemservices

deployer creates a systemservices instance, deploy all the services and inject them in the systemservices instance and then return this instance

that's at least the way I see it but definitely open for something else as long as it fixes this issue

[ilgooz]

Yes... That's right, it seems I missed that part.
About the solution, you already know what I think, that is on the forum proposal. I'm leaving the decision to team.

[ilgooz]

For the deployer initializing system services part, I don't agree with it. Because it doesn't make sense a pkg called deployer to init sytemservices pkg. systemservices should be in the parental position.

[krhubert]

For the deployer initializing system services part, I don't agree with it.

👍

Deployer should be only for deploying and that it. System services should deploy them

The deploy pkg you're proposing is similar to pkg that I proposed on from but with the name native.
You answered as "how many layers API of should be there?" and we went with the mapping idea of yours.

The API name is so misleading. I propose to create deploy package (that's it) - this 3 functions could be even in service package itself 'DeplofFromXXX'. It isn't new api/sdk/native etc just a package. Not an other layer or solution. Jsut to move functions from one place to the other - simple as that.

[ilgooz]

The API name is so misleading. I propose to create deploy package (that's it) - this 3 functions could be even in service package itself 'DeplofFromXXX'. It isn't new api/sdk/native etc just a package. Not an other layer or solution. Jsut to move functions from one place to the other - simple as that.

As I told in the proposal on forum, if a piece of code is ending up with import cycles it's something serious and we should rethink about changing the architecture and composition of the packages. Any attempt to solve import cycles without reorganizing the architecture is just going to be a workaround.

We wen't with the mapping workaround to solve one side of this import cycle problem and here we see that the problem remains with a new shape.

You're proposing to create a new pkg that has funcs to deploy and start services. Let's think that we did create a new pkg that exactly does that.

Yes, it will solve this side of import cycle problem and it'll bring a convenient structure.
If we apply the same thing and add more funcs to this pkg to execute tasks and listen for it's results, we'd also be solving the first side of this import cycle problem that happened for systemservices/* packages(wrappers) which would save us doing mappings.
Now we're talking about the same solution to solve this side of the import cycle problem where we can solve it for both sides by having the native pkg.

Now, if we do both mapping and have this deploy pkg of yours, this means we're doing multiple workarounds in different places where it can be solved by just having a proper pkg composition with the native pkg.

Today we need to use service deployment, starting, executing tasks and listening for task results functionalities from api pkg in system services and we're literally doing workarounds to access these simple features. And if in future, we need to access other features like getting service info, listing services etc. from the api pkg, we'll even need to do more workarounds. To avoid such workarounds like this we need a 3rd pkg to talk to like native pkg.

About adding start and deploy funcs directly to the service pkg, same is already discussed before by having the Manager type in the service pkg to cover these kind of stuff and we decided to not have it and put all the code under api pkg. api pkg chosen as the provider of high level functionalities of Core to consumers that wants to interact with Core.

We really had a long discussion about this import cycle problem on Forum, Github Issues, Discord and weekly meetings. And I feel like, I don't have more to contribute to this discussion because it's not going anywhere.

[antho1404]

I added one commit with what I was thinking with this system services deployer and the system services that contains just the list of the different services that the deployer inject into it.

This solves the cyclic dependency and I feel like that we have 2 system services packages that have specific responsibilities (one is the "database", the other is the one that fill this "database").

With that we can also have different ways to fill this system services. Here we deploy based on directories but if we find a way to release the version with a seed of a database with some already deployed system services we could just register these services based on the service database.

This commit can be reverted if it's really doesn't make sense, I just wanted to expose my solution with code because it seems I cannot explain it correctly during meetings.

[ilgooz]

Are we merging this one? @mesg-foundation/core please review. Before taking any action, I'd like to have conclusion about this #559 (comment) one as well. Because it might effect the solution here.

[krhubert]

This one for sure remove import cycle but I dont like the fact that there is deploying inside main. But I'm ok to approve it jsut to move on. But still I think the deploying should be done inside systemservices (wihtout api usage)

[NicolasMahe]

I approve this PR and also I made some minor changes:

• Fixing the test
• Removing a panic
• Move the not found error to systemservices
• Fix some text