Hi Andi, I like your solution because ... ... 1) It poses a recommended way as Default ... 2) It allows customization at solution level to meet relevant needs
@Dan, thank you for consideration w.r.to migration. It will definitely help my team because currently we are using 1.16.x in production and as soon as stable 2.x is available we will be upgrading to it. Thank you, Jayesh On Sat, Nov 17, 2018 at 4:48 PM Dan Haywood <[email protected]> wrote: > 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> > > > 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 <[email protected]> > 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 <[email protected]> > > > > 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 <[email protected]> > > > > 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 <[email protected]> > > 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 <[email protected]> > > 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 < > > > > > > > > [email protected]> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > No problem > > > > > > > > > > > > > > > > > > > > On Mon, Oct 29, 2018 at 2:47 AM Jayesh Prajapati < > > > > > > > [email protected]> > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > No problem from my side. > > > > > > > > > > > > > > > > > > > > > > @Anisha ... Do you see any problem? > > > > > > > > > > > > > > > > > > > > > > On Sun, Oct 28, 2018, 16:28 Patrick Pliessnig < > > > > > [email protected] > > > > > > > > > > > > > > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
