Hi Andi I think introducing this SPI is a good idea, as you say it will cover all use cases. And being very clear about it being immutable and NOT being a domain service is also correct, IMO.
I do still think that the default framework-provided implementation should support both PROTOTYPING env var and also "isis.deploymentType" as a system property. The latter provides a better migration path for 1.x users. But if you just want to do PROTOTYPING env var for now, that's fine; maybe I'll add in system property support later. Thx Dan On Sat, 17 Nov 2018 at 09:40, Andi Huber <ahu...@apache.org> wrote: > Thanks everybody participating in this discussion! > > I would like to bring this topic to a conclusion, especially because I'm > again investing hours of troubleshooting, because there are currently bugs > and inconsistencies with the v2 branch, when the framework evaluates itself > as being in prototyping mode or not. > > Summarizing my motivation: > > 1) PROTOTYPING true/false needs to be available to the framework before > domain-services are bootstrapped, hence the decision cannot be made with > such a service, that would be too late. > > 2) PROTOTYPING true/false should be immutable within the life-cycle of an > application, which technically can be perfectly done with system env. vars. > > Here's what I'm gonna do: > > I'll introduce an Interface 'DeploymentTypeProvider' that handles the > PROTOTYPING true/false decision. This is not a DomainService but a SPI! Any > implementation of this should not rely on the IsisConfiguration or any > DomainService, since these are only available later in the bootstrapping > phases. The framework will provide a default implementation utilizing the > concept I described earlier in this discussion (system env. var only). > > I'll leave it up to you to provide your own implementations of > 'DeploymentTypeProvider', which will be plugable via the ServiceLoader [1] > mechanism as was introduced with Java 7. > > Hope that serves all use-cases and btw. nothing is set in stone, we can > always change this later, if there are good reasons to do so. > > Cheers, Andi! > > [1] https://docs.oracle.com/javase/7/docs/api/java/util/ServiceLoader.html > > On 2018/11/01 20:31:51, Brian K <harvestmoon...@gmail.com> wrote: > > My experience with environment variables setting java system properties > to > > affect the application's behavior comes from Tomcat and JBoss. Their > > startup scripts set the java system properties of the java runtime from > the > > environment variables *if* they are set. The java system properties > > override any other default configured in the application from code or > > property files. To me, it doesn't make sense to ignore a property given > > explicitly on the command line in favor of an environment variable. > > > > Also, I found this discussion[1] that referred to the java docs [2]. The > > place for preferring environment variables, according to the java docs, > is > > if you are spawning a new process that does not have access to the java > > system properties. > > > > [1] > > > https://stackoverflow.com/questions/14026558/when-to-use-environment-variables-vs-system-properties > > [2] > > > https://docs.oracle.com/javase/7/docs/api/java/lang/System.html#EnvironmentVSSystemProperties > > > > > > > > > > On Wed, Oct 31, 2018 at 8:00 AM Jayesh Prajapati <jayesh...@gmail.com> > > wrote: > > > > > Hi Andi, > > > > > > My thoughts are towards jvm system properties because you need > environment > > > variable in case DOCKER only, which can be easily managed internally by > > > framework. > > > > > > I think, if we follow below for settings/option in framework then it > will > > > work for all use cases > > > > > > First Layer -> Default values for settings at code level > > > Second Layer -> Framework properties file that can override First Layer > > > Third Layer -> system property which can override First and Second > layers > > > Last Layer -> Configuration at database level that can override all > layers > > > (Applicable to specific settings that are used after application has > > > started) > > > > > > For DOCKER, it should be possible to supply desired value as > ENVIRONMENT > > > variable, however, internally image will supply relevant value in the > form > > > of system property using -D jvm option. > > > > > > I see the logic in framework as below ... > > > ... 1) if jvm property 'isis.deploymentType' is set then use value of > this > > > as deployment type literal > > > ... 2) else check if IsisConfiguration has value of isis.deploymentType > > > then use it as deployment type literal > > > ... 2) else check if environment variable ' DEPLOYMENT_TYPE' is > available > > > and use its value as deployment type literal > > > ... 3) else make use of ''PRODUCTION" as deployment type literal > > > (DEFAULT) > > > ... 4) Based on this deployment type literal lookup correct > > > DeploymentType > > > > > > My vote is -> As much possible try and avoid use of DEPLOYMENT_TYPE > > > > > > Thanks, > > > Jayesh > > > > > > On Wed, Oct 31, 2018 at 2:28 PM Andi Huber <ahu...@apache.org> wrote: > > > > > > > I think, this is an excellent opportunity for us to find out, what we > > > want > > > > to be ‘best practice’ and therefore can safely recommend to our > users. > > > > > > > > System-environment-variables do have one advantage over any other, > they > > > > are immutable for any operating system process that wants to read > them. > > > > (Hence I’m an advocate for having only this option.) > > > > > > > > Here would be my suggestion ... > > > > > > > > 1) Command-line > > > > > > > > For the command-line, in general simply prefix your command with ‘set > > > > PROTOTYPING=true|false’: > > > > > > > > set PROTOTYPING=true ; mvn jetty:run > > > > > > > > 2) IDE > > > > > > > > Eclipse and (I believe others likewise) allow for ‘Run > Configurations’ to > > > > be setup with system-environment-variables. That applies to the > launching > > > > of processes or test-runners from within the IDE. > > > > > > > > 3) Docker > > > > > > > > Docker allows for Docker images to be ‘primed’ with > > > > system-environment-variables using this entry in the image build file > > > > (Dockerfile): > > > > > > > > ENV PROTOTYPING true > > > > > > > > But this value can be overwritten, when you actually create your > Docker > > > > container: > > > > > > > > docker run -ePROTOTYPING=true|false my-image > > > > > > > > --- > > > > > > > > Question is, are these options flexible enough, to serve all > use-cases? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 2018/10/30 17:21:27, Dan Haywood <d...@haywood-associates.co.uk> > > > wrote: > > > > > Hi Andi / all, > > > > > > > > > > I was thinking a little more about this. > > > > > > > > > > For those of us who *are* on Docker, using the ENV keyword in the > > > > > Dockerfile [1] isn't really appropriate, because it shouldn't be > > > > necessary > > > > > to rebuild the image when moving from dev (prototyping=true) to > prod > > > > > (prototyping=false). > > > > > > > > > > That said, if using Docker without a swarm then docker run allows > the > > > > > environment variable to be specified in the command line (-e > option), > > > > which > > > > > is a good alternative. > > > > > > > > > > Or, if using Docker with a swarm, then the environment can be > specified > > > > in > > > > > the stack file (one stack file per environment), which also seems > okay > > > > [3]. > > > > > > > > > > So far, then, I'm still in agreement that there's no need for > > > > > isis.deploymentType to be a configuration property. > > > > > > > > > > But, building on Brian's slight confusion, it occurs to me that it > > > might > > > > be > > > > > useful to also support for prototyping mode to be read as a system > > > > > property. For backwards compatibility, I suggest that the system > > > > property > > > > > is named "isis.deploymentType". Therefore one could use: > > > > > > > > > > mvn -Disis.deploymentType=PROTOTYPING jetty:run > > > > > > > > > > or, if using org.apache.isis.WebServer main class in an IDE, then > > > > similarly > > > > > invoke that with a system property (easily specified in the IDE run > > > > dialog). > > > > > > > > > > ~~ > > > > > So, in conclusion, what I'm suggesting is that we do remove > support for > > > > > reading isis.deploymentType as a config property, but support > reading > > > > both > > > > > isis.deploymentType as a system property, and PROTOTYPING as an > > > > environment > > > > > variable. > > > > > > > > > > Thoughts? > > > > > Dan > > > > > > > > > > > > > > > > > > > > [1] https://docs.docker.com/engine/reference/builder/#env > > > > > [2] > > > > > https://docs.docker.com/v1.7/reference/run/#env-environment-variables > > > > > [3] https://docs.docker.com/compose/environment-variables/ > > > > > > > > > > On Mon, 29 Oct 2018 at 19:29, Brian K <harvestmoon...@gmail.com> > > > wrote: > > > > > > > > > > > Thanks for clarifying! My concern is that flexibility would be > lost > > > > if you > > > > > > remove a configuration path. I thought the common practice was > to > > > > have the > > > > > > java system property be what drives the application's behavior, > and > > > > that > > > > > > property is loaded from the environment if it isn't set > explicitly at > > > > the > > > > > > command line. The web.xml file can reference the java property > with > > > > syntax > > > > > > like <param-value>${prototyping}</param-value> > > > > > > > > > > > > An interesting (some would use another word) puzzle would evolve > when > > > > > > one developer runs a packaged war file and gets the prototyping > menu > > > > > > while another developer who runs it can't get that prototyping > menu > > > to > > > > > > display until he figures out he must set an environment > > > > > > variable to see it. > > > > > > > > > > > > -Brian > > > > > > > > > > > > > > > > > > On Sun, Oct 28, 2018, 10:02 PM Andi Huber <ahu...@apache.org> > wrote: > > > > > > > > > > > > > Hi Brian, > > > > > > > > > > > > > > not sure if I understand all your questions, but to clarify: > > > > > > > > > > > > > > In our context PROTOTYPING=true is a system environment > variable > > > > [1][2] > > > > > > > for which the running operating system accounts responsible. > You > > > can > > > > set > > > > > > > such environment variables in multiple ways and note, this > does not > > > > > > require > > > > > > > Docker! > > > > > > > > > > > > > > Also note: mvn -Dkey=value does not set an environment variable > > > (this > > > > > > does > > > > > > > set a system property, which is specific to the Java runtime) > > > > > > > > > > > > > > Does this answer your questions and concerns? > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > > > > https://docs.oracle.com/javase/8/docs/api/java/lang/System.html#getenv-java.lang.String- > > > > > > > [2] https://en.wikipedia.org/wiki/Environment_variable > > > > > > > > > > > > > > On 2018/10/29 04:41:40, Brian K <harvestmoon...@gmail.com> > wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > Two points to consider: > > > > > > > > > > > > > > > > Being new to this platform, it was very useful to see the > > > > prototyping > > > > > > > menu > > > > > > > > when prototyping with 'mvn jetty:run'. This change would > require > > > > 'mvn > > > > > > > > -DPROTOTYPING=true jetty:run' ? Would jetty console stop > > > > functioning > > > > > > > with > > > > > > > > a simple launch with this change? > > > > > > > > > > > > > > > > Please don't assume Docker is available. My lan uses the > same > > > > netmask > > > > > > > that > > > > > > > > Docker uses, making it hard to make use of this tool. > > > > > > > > > > > > > > > > -Brian > > > > > > > > > > > > > > > > > > > > > > > > On Sun, Oct 28, 2018, 12:27 PM Stephen Cameron < > > > > > > > steve.cameron...@gmail.com> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > No problem > > > > > > > > > > > > > > > > > > On Mon, Oct 29, 2018 at 2:47 AM Jayesh Prajapati < > > > > > > jayesh...@gmail.com> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > No problem from my side. > > > > > > > > > > > > > > > > > > > > @Anisha ... Do you see any problem? > > > > > > > > > > > > > > > > > > > > On Sun, Oct 28, 2018, 16:28 Patrick Pliessnig < > > > > p.pliess...@gmx.net > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > No problem > > > > > > > > > > > > > > > > > > > > > > Am 28.10.2018 um 10:29 schrieb Andi Huber: > > > > > > > > > > > > Hello everyone! > > > > > > > > > > > > > > > > > > > > > > > > Dan and I discussed [1], whether we should remove the > > > > config > > > > > > > option > > > > > > > > > > > 'isis.deploymentType'. > > > > > > > > > > > > > > > > > > > > > > > > Remember, we are having this, in order for > developers to > > > > > > decide, > > > > > > > > > > whether > > > > > > > > > > > an Apache Isis App should be deployed in PROTOTYPING > mode > > > or > > > > not > > > > > > > > > > > (PRODUCTION). > > > > > > > > > > > > > > > > > > > > > > > > In the advent of Docker, we had to introduce support > for > > > a > > > > > > system > > > > > > > > > > > environment variable (PROTOTYPING), that serves the > exact > > > > same > > > > > > > purpose. > > > > > > > > > > > > > > > > > > > > > > > > Now, instead of having to support 2 possible ways to > > > > configure > > > > > > > the > > > > > > > > > > > deployment type, we would like to discontinue the first > > > one. > > > > This > > > > > > > > > > relieves > > > > > > > > > > > us from the burden of having to decide, which one has > > > > precedence > > > > > > > over > > > > > > > > > the > > > > > > > > > > > other, and also having to document this somewhere. As a > > > > positive > > > > > > > > > > > side-effect, developers are not encouraged to provide > > > > different > > > > > > war > > > > > > > > > files > > > > > > > > > > > for different deployment scenarios. > > > > > > > > > > > > > > > > > > > > > > > > So we are asking you, do you have any strong > objections > > > > > > regarding > > > > > > > > > this > > > > > > > > > > > move? > > > > > > > > > > > > > > > > > > > > > > > > Cheers Andi! > > > > > > > > > > > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/isis/commit/4ea76029e097e3e8b94f5602ca430dfcd6ee9dac > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
