Conversation
|
that was easier than I expected! LGTM |
|
Verified on QEMU/i7 against foundriesio/aktualizr-lite#9, it worked for me. |
| device.put("overrides.pacman.docker_apps_root", "\"" + apps_root + "\""); | ||
| if (!options.docker_apps.empty()) { | ||
| device.put("overrides.pacman.docker_apps", "\"" + options.docker_apps + "\""); | ||
| string apps_root = options.sota_config_dir + "/compose_apps"; |
There was a problem hiding this comment.
@doanac I wonder if we should call this dir compose_apps as well as the command line param as compose-apps, maybe leave it as docker* since docker term is more common (I am subjective here) or maybe docker-compose-apps?
There was a problem hiding this comment.
I've been leaving "docker" out because:
- docker-compose-apps is a mouthful
- docker-apps and docker-compose-apps look really similar.
I almost wonder if we should call this cli argument "apps". Then the parameter is good either way.
Another thing I realized we need to check for: We should only enable this new logic if "DOCKER_COMPOSE_APP" is defined in the factory-config.yml.
There was a problem hiding this comment.
I almost wonder if we should call this cli argument "apps".
+1, What about the configuration variables in sota.toml and the apps directory name?
There was a problem hiding this comment.
We should only enable this new logic if "DOCKER_COMPOSE_APP" is defined in the factory-config.yml.
Do you mean taking into account it in the lmp-device-register recipe? I suppose a package manager type should be defined anyway. This is actually not a new logic just renaming.
There was a problem hiding this comment.
@doanac let's chat and make a decision on this.
There was a problem hiding this comment.
Do you mean taking into account it in the lmp-device-register recipe?
Correct. We'd need to set this in our lmp build logic around here:
and then the recipe could key off that value if it exists.
There was a problem hiding this comment.
What about the configuration variables in sota.toml and the apps directory name?
I don't have a strong opinion on the variable names. As per the "apps" directory. I noticed that on my local test device I had simply named the directory /var/sota/compose. I kind of like just calling it "compose" - once its on disk its just vanilla compose. Its only the transports that's doing something new.
|
@doanac Renamed compose-apps to apps |
Signed-off-by: Mike Sul <[email protected]>
3b2c4fd to
bc452db
Compare
|
looks good. I think we should get @ricardosalveti to take a look as well. |
|
@ricardosalveti Please, review it |
Signed-off-by: Mike Sul [email protected]