Thank you for the input.
@Owen, just is not a pressing issue that I have, I was just in the code, saw
the test, that seemed "incorrect" in the determination of version and/or
artifact name of a jar file.
@Dan, I agree with you I don't want to introduce a breaking change for no
reason. Bad experience.
What is evident from some testing that I've been doing, is that we do have an
undocumented feature and format that Geode will accept. So maybe Step1 in this
case is to document the file format that we currently "support". As it is VERY
close to the one that I've been suggesting. In addition, we need something that
resembles consistency.
E.g
The system will "detect" similar artifact names if the following formats are
used:
{artifact_name}.jar
{artifact_name}-0.0.0.jar
But not when using:
{artifact_name}.v1.jar
Which means that in 1 case the artifact name is determined and can be
"upgraded" and in the other not. But maybe going forward this file format is
documented as a preferred format. Maybe that is a gentler way of changing the
format.
But I think for future use, there should be no restriction on filename format.
As consistency of experience is important. A more explicit jar deploy command
is required, similar to that of maven's install jar format.. artifact name +
version, explicitly required when deploying the file. Deploying from maven with
dependencies does sound inviting.
But thank you for all the input.
--Udo
On 10/10/20, 8:16 AM, "Dan Smith" <[email protected]> wrote:
I also don't see a pressing need for a breaking change here. It's unlikely
the current behavior is going to cause any problems for users with "standard"
jar file names. On the other hand, having failures or new, weird classpath
issues on upgrade for users with non-standard jar names doesn't seem like a
good UX.
I do like the idea of moving forward to a better way of specifying artifact
names and versions than parsing a file name. Maybe jigsaw module names, or
maybe supporting deploying a maven artifact (and its dependences??).
-Dan
________________________________
From: Owen Nichols <[email protected]>
Sent: Wednesday, October 7, 2020 7:24 PM
To: [email protected] <[email protected]>
Subject: Re: [DISCUSS] Supported filename convention for Deploy Jars
functionality
I do not feel that we need to restrict the names of jar files that users
may deploy. GEODE-7436 does the most reasonable thing in the vast majority of
cases, but a user is always free to override the default logic, either by
manually un-deploying some existing jar before deploying a new one, or renaming
one of them if they truly require both of two lookalike jars deployed
simultaneously. This is consistent with the ideal of making simple things
easy, but hard things possible.
I fail to see a problem here that rises to the level of justifying
"breaking changes" with "no transition ability".
@Udo, the work you are citing in conjunction with this appears to be
related to classloader changes. Can you clarify whether your proposed
restrictions on jar names are essential to implement your classloader changes,
or just an unrelated thing you happened to notice in the course of that work?
On 10/7/20, 4:45 PM, "Udo Kohlmeyer" <[email protected]> wrote:
@Owen, not sure if I'd use "harmless". I'd use "unlikely", rather than
"harmless", as it can still have harmful consequences.
I think the "intuitive" nature of the versioning means we have to have
a standard jar file format, so that the system can intuitively understand that
"some-jar-0.2" is the update to "some-jar-0.1". There HAS to be some form of
format for the auto-version-update to work, otherwise one could never compare.
Maybe I'm just trying to be more explicit. That way, any implementation
of the "deploy jar" functionality can rely on a single fact, that the jar files
will be in a known format.
The alternative is maven-like in behavior... then we don't have to deal
with explicit file formats. When a jar is deployed, the feature behaves more
maven-like, and "artifact name", "version" and file path is required. That way,
the jar file does not ever have to meet the required format. All information
required has been provided by the user. Removes the false-positives that could
be introduced.
As there is no public API here, the only change we would have to deal
with is the gfsh scripting, which I assume is not so horrible.
Anyway.. just thoughts..
--Udo
On 10/8/20, 9:57 AM, "Owen Nichols" <[email protected]> wrote:
The goal of the GEODE-7436 change is that when user deploys
some-jar-0.1, then later deploys some-jar-0.2, we will do the intuitive thing
and treat it as an update. In Geode 1.11 and earlier, we instead wound up with
*both* deployed, which is bad!
In the weird example below of spark-network-common_2.11-2.3.1.jar,
it is harmless that we stem it as spark-network-common_2 (*if* for some very
crazy reason a user NEEDS both spark-network-common_2.11-0.0.0.jar and
spark-network-common_2.12-0.0.0.jar deployed side-by-side, they can simply
rename the jar to something more sane)
On 10/7/20, 3:45 PM, "Anthony Baker" <[email protected]> wrote:
Given the wide variety of filenames possible do we even need a
classification scheme? IOW, why not just take what the user gives us and say
thank you :-). Is this restriction imposed by our *implementation* choices?
Anthony
> On Oct 7, 2020, at 3:24 PM, Jinmei Liao <[email protected]>
wrote:
>
> Wait, that reason doesn't make much sense either.
Dale/Darrel, do you remember why we did what we did?
>
> On 10/7/20, 3:12 PM, "Jinmei Liao" <[email protected]> wrote:
>
> I believe we did this for a reason, can't remember exactly
what though. Most probably drive by user's existing filenames. I believe we are
probably concerned that user's jar name might contain "_" or "-" themselves,
like common-logging.jar etc. So we had to resort to finding the first "."
followed by a digit to determine where the version pattern begins.
>
> On 10/7/20, 1:44 PM, "Udo Kohlmeyer" <[email protected]>
wrote:
>
> Hi there Geode Dev List,
>
> Whilst doing work on
GEODE-8466<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FGEODE-8466&data=04%7C01%7Cudo%40vmware.com%7C1a28265ca33a495332c108d86c989840%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637378749984919175%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=hP8Y8UCyv27YDzOKX3ILJyt0ZLH%2Fn6va2n03NYvrQtc%3D&reserved=0>
and looking at the functionality that the
ClassPathLoader.java<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fblob%2Fdevelop%2Fgeode-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fgeode%2Finternal%2FClassPathLoader.java&data=04%7C01%7Cudo%40vmware.com%7C1a28265ca33a495332c108d86c989840%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637378749984919175%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=ldscoEINR7clDjfT8sfuHFxO9gN6EtU1i%2BI9GiUsdao%3D&reserved=0>,
JarDeployer.java<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fblob%2Fdevelop%2Fgeode-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fgeode%2Finternal%2FJarDeployer.java&data=04%7C01%7Cudo%40vmware.com%7C1a28265ca33a495332c108d86c989840%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637378749984919175%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=YQfTloae6Bt9l38xVIPyEd7ZPBfRUzaBpN7IfxBU0q4%3D&reserved=0>
and
DeployedJar.java<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fblob%2Fdevelop%2Fgeode-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fgeode%2Finternal%2FDeployedJar.java&data=04%7C01%7Cudo%40vmware.com%7C1a28265ca33a495332c108d86c989840%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637378749984919175%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=hjhgTcaP%2F6cYE6xzWhHvJ0qfgH6QiQex2zokaxYB2Zo%3D&reserved=0>
provide around the “Deploy Jar” functionality, we came across some interesting
“supported” filename patterns.
>
> According to the
JarDeployerFileTest.java<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fblob%2Fdevelop%2Fgeode-core%2Fsrc%2FintegrationTest%2Fjava%2Forg%2Fapache%2Fgeode%2Finternal%2FJarDeployerFileTest.java&data=04%7C01%7Cudo%40vmware.com%7C1a28265ca33a495332c108d86c989840%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637378749984929169%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=aJFpho3sNyl9oQaq%2F2w0h8VhdBCBQJ2byWIkKB7D%2Fww%3D&reserved=0>
the “supported” formats are as follows:
>
>
assertThat(JarDeployer.getArtifactId("abc.jar")).isEqualTo("abc");
>
assertThat(JarDeployer.getArtifactId("abc-1.jar")).isEqualTo("abc");
>
assertThat(JarDeployer.getArtifactId("ab.c.1.jar")).isEqualTo("ab.c");
>
assertThat(JarDeployer.getArtifactId("abc.v1.jar")).isEqualTo("abc.v1");
>
assertThat(JarDeployer.getArtifactId("abc-1.0.snapshot.jar")).isEqualTo("abc");
>
assertThat(JarDeployer.getArtifactId("abc-1.0.v1.jar")).isEqualTo("abc");
>
assertThat(JarDeployer.getArtifactId("spark-network-common_2.11-2.3.1.jar"))
> .isEqualTo("spark-network-common_2");
> Which don’t make any sense. As the generally accepted
norm for a version jar file would be: “<artifact name>[ - <major> . <minor> .
<patch> - <Release Tag> ] .jar”. (note the syntax in red)
>
> I want to suggest that we DISCONTINUE supporting all
jar name formats other than the one mentioned above IMMEDIATELY. As the
supported name format is just “funky” but also wrong and can lead to
misclassification of the artifact name…. as some of you with a keen eye would
have spotted already 😉
>
> For those who did not spot the mistake…
“spark-network-common_2.11-2.3.1.jar” is incorrectly classified and has the
WRONG artifact name. As “spark-network-common_2.11” is the correct artifact
name NOT “spark-network-common_2”!
>
> I would like to introduce this change with
GEODE-8466<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FGEODE-8466&data=04%7C01%7Cudo%40vmware.com%7C1a28265ca33a495332c108d86c989840%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637378749984929169%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=qHM5eapeHFKJqcRr9GFLqR3ktgMLdXVA13RcMjcToNE%3D&reserved=0>.
This would be a “breaking” change, but we should change this sooner than
later. There is no transition ability here, as it would be too hard to have
Geode support both, as there is no simple way for the system to decide if the
name conforms to the “correct” format or not.
>
> DISCUSS!!!
>
> --Udo
>
>
>
