Clarify location of spec artifacts #221
Comments
|
It's always been confusing where the location of config files is in a WAR file - JPA persistence.xml is supposed to be in classes/META-INF but CDI's beans.xml should be in WEB-INF/beans.xml. Though, I haven't seen a solution where a file is expected in the root I don't mind too much but MP Specs should be consistent and support same options. All the following options meet that:
From the above, my preferred solution is 3. or if MP Config wouldn't agree then option 1. The option 2. is supported by OpenAPI now but I don't think it's the best solution and I discourage from adopting it for MP Config and rather make it obsolete for OpenAPI. |
|
thanks a lot for outlining these different alternatives @OndrejM - it helps to see them all in one place. Part of the motivation for the Perhaps going forward in the new |
I wonder, is that a real scenario? If you can touch |
|
From #222 MP Open API currently specifies [war root]/META-INF for certain files to be placed as well as a MP Config properties file.
I though this issue (#221) was different, since this one just asks for clarification, while #222 specifically asks for deprecation, but if everything can be handled here that's fine too. |
|
thanks @arjantijms - this issue will decide which way we go forward in |
|
@arjantijms / @OndrejM - one such example is Tomcat's usage of context.xml, which sets per-web-app configuration at the |
That one is however not a configuration file that belongs to a spec implemented by Tomcat and/or a spec module bundled in WEB-INF/lib. context.xml is more akin to domain.xml in Payara and standalone.xml in JBoss. You typically put that file outside the archive (in a server folder). Tomcat in fact treats it like that, as in it copies context.xml from the war using low level code to the usual location outside in the server, but only if there isn't already a context.xml there. So although I do appreciate you found one example, this is a highly exceptional one really. |
It demonstrates there's precedence for using Also, the fact that Java SE uses So - although |
|
I'll check the liberty example to see if they actually mean I don't think Tomcat is a valid example really. It doesn't process or read
I don't quite agree with that. The /META-INF/services for jars is well known and often used in wars, and that's via
The fact that it's possible and even that there might be some (weak?) precedence, does beg the question whether we -should-? Obviously it has proven to be possible for JAX-RS to use |
|
One thing I'd like to make sure is that OpenAPI doesn't define only locations that are specific to |
|
+1 @kenfinnigan I think a lot of the arguments in this thread are geared solely for Since the OAS3 doc is in that location, the MP OpenAPI spec allows the flexibility to put the OpenAPI config in that directory too - in addition to (not replacing) the other valid MP Config locations. |
|
Hi,
On Mon, May 21, 2018 at 1:05 PM, Ken Finnigan ***@***.***> wrote:
One thing I'd like to make sure is that OpenAPI doesn't define only
locations that are specific to WAR files, as JAR is still a perfectly
valid deployment package type
You mean an EJB jar? Are these supported by MicroProfile?
… —
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#221 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AC5XTjN3V0TgWj9t5OloIO1xcEvafJ8Cks5t0q1ygaJpZM4UFgtF>
.
|
|
Hi,
On Mon, May 21, 2018 at 2:26 PM, Arthur De Magalhaes < ***@***.***> wrote:
+1 @kenfinnigan <https://github.com/kenfinnigan>
I think a lot of the arguments in this thread are geared solely for WAR,
which is one of the deployment patterns, but not the only one. Having the
OAS3 doc in the <root>/META-INF allows it to apply to any deployment
pattern derived from jar (WAR, OSGi app, Jar, etc), which is one of the
reasons why Liberty chose that location even for the Swagger document
support back in 2016.
Perhaps you're mistaken with "META-INF on the classpath"?
"META-INF on the classpath" works everywhere. It's <root>/META-INF for an
(EJB) jar, and WEB-INF/classes/META-INF for a war.
At any length, I thought MicroProfile only supported JAX-RS and with that
WARs, but I might have missed something.
Kind regards,
Arjan
… Since the OAS3 doc is in that location, the MP OpenAPI spec allows the
flexibility to put the OpenAPI config in that directory too - in addition
to (not replacing) the other valid MP Config locations.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#221 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AC5XTgVa4Wdhx_5-FP2_XCbWpBGJZcN6ks5t0sB2gaJpZM4UFgtF>
.
|
|
No, I mean any old JAR. MicroProfile does not dictate the packaging format as WAR or JAR, it could be either, and certainly some implementations support one or the other or both. My concern was that it seemed to be pointing towards a WAR specific location being mentioned in the spec, which is I said that it needs to work for WAR or JAR structures |
|
Consensus was reached today at the hangout that the current behaviour is correct. I will assign this to myself to clarify in the spec. |
I wonder if that should not be an overal concern then, since the other MP specs as far as I know don't really say anything about that, do they?
Which is really simple to solve, by using the META-INF folder that's in the class path. That will basically resolve automatically to whatever physical location is valid for that in whatever archive.
I wonder, who was present in that hangout and based on which arguments was that consensus reached? If it were the same people who originally okay'ed that to be in the spec in the first place, it's perhaps not a surprise they reached consensus about being correct themselves. I feel it might be better to bring this to the broader attention of an architecture board in MP. |
|
@kenfinnigan @arthurdm, you're missing a few very important points:
Therefore specifying the location as "META-INF" on classpath is more correct and works for both JAR and WAR files. For JAR files, it would be also valid to specify that "META-INF" is in root, but this wouldn't work for WAR files. I'm not suggesting we need to treat WAR files in a special way but we should specify a location that doesn't break WAR files. Specifying location as "META-INF" on classpath doesn't break WAR files while specifying the location as "META-INF" in the root of the package does because it exposes files to the outside world. It would be convenient if WAR files used "WEB-INF" location as an alternative, but it's not necessary if you don't want to mention WAR files in the spec. |
|
As a result, an alternative location in This section of the spec needs to be modified:
Should be
And the PR #220 should be accepted, or better, a new test case should be created for the new location, while the old one deprecated. |
|
thanks for the comments guys. The conclusion from the hangout today is that:
|
|
Thanks for the reference to the document. This clarifies that Ivan Junckes Filho from Tomitribe, Eric Wittmann from Red Hat, and yourself reach that consensus (I'm not sure who Marc S is though). But since it was a video hangout, it doesn't reveal which arguments where used to reach that consensus. I'm still very curious why anyone would think So would great if you could tell us a little about which actual arguments were used. |
|
Marc S is from Red Hat, and Andy G from Tomitribe was also present (updated attendance now). The arguments used were the same described along this thread - we took into consideration the arguments in favour and against the current solution, so we did have a fair conversation about it. I will go ahead with a PR for the agreed direction, but if you still feel strongly against it, please join the next hangout on June 18, 10am EST and we can chat. |
|
I've raise eclipse/microprofile#46 for the Arch. board to address this globally. To summarize the discussion here, the only argument for keeping the current state is mentioned in here
I didn't find any other argument to support the decision, despite it was declared multiple times that the discussion in hangouts concluded with keeping the current state. I can easily counter that argument. For JAR files, the situation doesn't change with having artifacts in META-INF on classpath as it's the same location as Another (invalid) argument was that there's already a precendence in using root/META-INF. But all examples of this are vendor-specific (tomcat, liberty), while there's a strong precedence in existing specifications to use either META-INF on classpath (works in both JAR and WAR) or WEB-INF (in case of WAR). Adding another META-INF location for WARs causes confusion, as you can see in this StackOverflow question: https://stackoverflow.com/questions/17997731/maven-war-has-meta-inf-folder-in-two-places. The answer pinpoints the servlet spec, which on one hand allows root/META-INF and says it isn't visible to the outside world, on the other hand it states that this is reserved when using the standard Java archive tools and implies that it shouldn't be used in any other way. So the precedence for root/META-INF is much smaller than for other locations and this argument thus isn't valid at all. On the other hand, there are many arguments for changing the expected location to META-INF on the classpath instead of in the root of a package:
It's 5 valid arguments for changing the current behavior against 1 valid argument to keep it. |
|
In addition to agreeing with Ondrej's points raised above, I would like to call special attention to this:
So far the discussion has given a distinct feel of "we've chosen it before, maybe not thought too much about it, but now that it's chosen it has been chosen". MicroProfile however promised to be fast paced, and part of this fast pace mean mistakes slip in. To counter this, it was also promised to be agile with correcting mistakes, as opposed to the Java EE process where things that slipped into a spec release can not really be changed anymore without moving the world and many years of waiting (typically 6 years at least). I personally think there's very little discussion needed about I assume the issue is solely with the idea of having to correct something in a spec, not about it actually being a mistake. As such I think this should not be an issue at all, as the MP process allows for things to be corrected without too mush hassle. |
|
For .war in CDI, beans.xml can be placed under WEB-INF\beans.xml or WEB-INF\classes\META-INF\beans.xml. If multiple files are found in both locations, an unportable behavior occurs. Both @struberg and I did not like it. Therefore, in MP Config, we simply said the microprofile-config.properties on the classpath will be loaded, which means WEB-INF\classes\META-NF\microprofile-config.properties for .war files or META-INF\microprofile-config.properties for .jar files. It is unusual to load files from META-INF\ folder under .war. I think in MP we should adopt what MP config has done - classpath only, which is clear and neat. If any vendor wants to load files from different locations, they are free to do so. |
|
And then .war/META-INF/*.properties seems to work across many/all(?) servers. |
|
@pilhuhn nop, |
|
@rmannibucau I guess I was trying to make the distinction between 'de facto' and 'de jure'. If |
|
I think this thread has focused too much on the EE classpath, specially for a So when figuring out the best place the store this static OAS3 doc we took in consideration the common denominator in all Java deployments, including EE, SE, Spring, etc, and As stated before, Liberty has been doing this for over 2 years now, and we have noticed that adopters of API-design first approach in the field usually have build-time tools which extract the OAS3 from Git, validate against the application (whichever that may be, |
|
@arthurdm well, actually no. MP is about embrassing EE and being reversed to JakartaEE so if you do something against EE like using vendor specific descriptors or locations like .war/META-INF then you block that process. Functionally there are well defined and known location to meet the same requirement as mentionned previously so no need to reinvent the wheel and loose users. |
Thanks for confirming my point. =) |
|
@arthurdm the most important part being the root spec of mp-openapi is mp so EE and it is an openapi integration in EE and not the opposite ;) |
|
Isn't your reasoning a variant of the age old: "There might be a person, deep in the jungles of Japan, who might want to use [insert EE tech] with Python/Spring/Ruby/What have you" In general that leads to very bland, and overly complex designs, for no good. (Python users would never want to use an API that's designed for Java). JSR 77 is a prime example. I don't mean this in any way as offence, so please bear with me, but if you like to do work for OpenAPI (OAS3) or Spring, wouldn't it be a better idea to work with or for them directly instead of polluting/compromising the EE targeted MP specs? |
|
I think we've heard a lot of opinions already. @arthurdm as a s pre c lead has his own opinion. But the role of spec lead is not to make ultimate decisions but drive collaboration and facilitate agreements. Arthur is failing in this respect. I'm sorry to say so, especially because I haven't met Arthur to have and I'm sure we'd have a friendly talk together. At this stage, the only reasonable option is to have a wider discussion in the MP community and ultimately vote on a decision among all MP committers where everybody has an equal vote, including spec leads. |
@OndrejM - we have discussed this topic in three different MP OpenAPI hangouts, where I invited the opinions of all members - so definitely opinions were heard and discussed. Just ask @EricWittmann or @ivanjunckes or @msavy or @AndyGee or @leochr I could have just ignored this issue, like it happens in many other specs (in or outside MP), but instead I gave it the appropriate attention - three hangouts, plus all the discussions here..which don't just reflect my opinions, it reflects the consensus reached at the various hangouts. We actually even agreed today (as a team) to defer this topic to whatever decision comes out of eclipse/microprofile#46 - so I definitely don't see any problems with the way this particular issue was handled. We discussed, invited opinions, had chats, reached an agreement to move forward. |
|
Ultimately no one on the MP OpenAPI hangout felt strongly enough about this issue one way or the other, so the agreement we reached was to defer to the decision of the arch. board. Also I noticed that the information included in eclipse/microprofile#46 is unclear (at least to me). So I'll update that in order for the board to make an informed decision. |
|
@EricWittmann But perhaps that was because all the people on that hangout were the same people who didn't care (or thought about it) in the first place? |
|
Very likely. And without anyone else joining the call, I think a reasonable decision was made: abide by the decision reached on issue eclipse/microprofile#46 Disagree? |
It's now probably for the best to wait for 46 then. Still it surprises me how those on the hangout would not feel strongly about this given all the arguments raised by so many different people. |
|
@arjantijms, it seems that your comment was add multiple times. Probably Github messed up, Github was messing up with me yesterday too. I deleted all the redundant comments. |
|
I agree, @EricWittmann with moving the resolution to eclipse/microprofile#46 and many thanks for clarification in that issue, you described the problem very well! Just one thing to add - it's not easy for all interested parties to join the hangout, especially at the same time. Therefore I wouldn't make a decision during any hangout if there's a wide dispute like this one. It's better to ask on the mailing list and let people vote. Or discuss this in gitter so that it's easier to join the discussion and also keep the history of the discussion. Hangouts should be for better coordination and not for making decisions. |
|
@OndrejM thanks, GitHub just has an outage in the middle of me posting that comment (https://twitter.com/githubstatus/status/1018911757245538304) |
This issue will take a look at the spec, decide what is the appropriate design, and further clarify the location of the various artifacts, such as the OpenAPI static file / snippet, and property files.
The text was updated successfully, but these errors were encountered: