System service instance#1087
Conversation
antho1404
force-pushed
the
feature/system-service-instance
branch
from
c381cb6 to
fcf6f8e
Compare
antho1404
force-pushed
the
feature/system-service-instance
branch
from
9c2dbc0 to
085b8b7
Compare
antho1404
force-pushed
the
feature/system-service-instance
branch
from
085b8b7 to
ff3eb5d
Compare
|
@NicolasMahe can you take care of ethwallet service |
NicolasMahe
left a comment
NicolasMahe
left a comment
There was a problem hiding this comment.
It's looking good!
I would suggest to simplify the override of the env of the system services. Instead of having to define one variable per env, we could pass ALL envs in one variable as a JSON encoded array. The Engine would have to decode it to start the instances.
Maybe we could go further and create a generic system based on JSON encoded string or a file that is transform to an asset during the compilation, that contains all info about the SystemService. Like this we will not have to update the config package to add or update system services. @krhubert suggested something like this on Discord.
This is definitely for another PR and it's not blocking for the next release.
| pushd $s > /dev/null | ||
| name=$(basename "$s") | ||
| varname="${name}" | ||
| LDFLAGS+=" -X 'github.com/mesg-foundation/core/config.${varname}Compiled=$(cat compiled.json)'" |
There was a problem hiding this comment.
also, why not compiling the service also in this script? like scripts/build-engine.sh is doing? because of optimization?
There was a problem hiding this comment.
yes because of optimization but not only, we should only have the compiled version of the service and if we have the compiled version there is no need of having the sources of the system service in the engine and in that case, the compiled version is a snapshot of the exact version we want from this system service that we can manually update with something like
mesg-cli service:compile https://github.com/mesg-foundation/service-marketplace > systemservices/marketplace.json
| Url: s.URL, | ||
| Sid: s.Definition.Sid, | ||
| Hash: s.Instance.Hash.String(), | ||
| Url: s.Definition.Source, |
There was a problem hiding this comment.
Is it useful to return the Source?
I think it will be nice to also have ServiceHash along side InstanceHash.
But anyway Hash = InstanceHash is the most important one 👍
There was a problem hiding this comment.
just didn't want to break the api and return what was initially returned. Let's remove it if needed in another PR
# Conflicts: # config/services.go # core/main.go # dev # scripts/build-core.sh
NicolasMahe
force-pushed
the
feature/system-service-instance
branch
from
eb9a5dc to
6ebc490
Compare
# Conflicts: # config/services.go # core/main.go # dev # scripts/build-engine.sh
Co-Authored-By: Nicolas Mahé <nicolas@mesg.com>
Co-Authored-By: Nicolas Mahé <nicolas@mesg.com>
2 participants
Dependency: #1085
service.Createandinstance.Createfunctionsdevscriptgo-assetsor something like thatfix #1115