Warn users when they import recipes or apps with target properties

roboconf / roboconf-platform

The core modules and the platform

Apache License 2.0

35 stars 11 forks source link

Warn users when they import recipes or apps with target properties #689

Closed cdeneux closed 8 years ago

cdeneux commented 8 years ago

Now, the target id is unique through all targets of all applications deployed.

If you have 2 applications based on a same import containing a target definition, the id of this target will not be unique if both applications are deployed on the same Roboconf instance.

A solution must be found to prevent this use-case, for example avoiding definition of target in an import.

vincent-zurczak commented 8 years ago

Graph import relies on the Maven plug-in. So, the Maven plug-in should introspect imported projects and verify they do not container target properties. A warning should be addressed in such cases.

cdeneux commented 8 years ago

IMO, a warning is not sufficient because it will be invisible in all traces generated by Maven.

Perhaps the solution could take place at the moment deploying the Roboconf application:

when deploying an application, if a target is embedded and already exists (deployed by another application, or by a target deployment), the deployment fails with message explaining that the current application must be reviewed to use Roboconf best-practices (ie, the target definition should be externalized),
so, if both applications are based on a same import, the import can be change to externalize the target and all will works fine.

This solution is more generic and works if two applications define the same target but don't use a common import.

vincent-zurczak commented 8 years ago

Once zipped, it is not possible to determine whether an application was built using imports or not. Reusable recipes are a build thing.

cdeneux commented 8 years ago

I think that you didn't understand what I mean.

When deploying an application, if it contains a target, the target will be deployed. If the target was previously deployed (by another application or by a target archive), an error occurs. So, the application deployment fails. In the error message, just explain to use Roboconf best-practices.

I'm no more sure that we should prevent this use case at build time.

vincent-zurczak commented 8 years ago

Since #690, the ZIP deployment already fails if it contains a target that was already deployed. Anyway, we should keep this issue to add a warning in the Maven plug-in, when we import another project.