@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" <onich...@vmware.com> 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" <bak...@vmware.com> 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 <jil...@vmware.com> 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" <jil...@vmware.com> 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" <u...@vmware.com> 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&amp;data=04%7C01%7Cudo%40vmware.com%7Ce4453349214a4092820c08d86b146981%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637377082704841015%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=CTxOpcNcxlH9ox5rTNlPsYZV9%2Fv1ywX9FnjWR1zcL1w%3D&amp;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&amp;data=04%7C01%7Cudo%40vmware.com%7Ce4453349214a4092820c08d86b146981%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637377082704841015%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=LQSfvmzjF%2FiJ9xlMhCtrFcbZh4lrcl2Idj3pj2B3UZg%3D&amp;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&amp;data=04%7C01%7Cudo%40vmware.com%7Ce4453349214a4092820c08d86b146981%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637377082704841015%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=RWEnHRxb%2BiNMV6DruQ2QyRITYIXOz5LYvvrxyGrHT1w%3D&amp;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&amp;data=04%7C01%7Cudo%40vmware.com%7Ce4453349214a4092820c08d86b146981%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637377082704851012%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=RnD4kMH5%2BmEj9JAMviLheHROmTyQig0En%2F3M2kpl%2B4M%3D&amp;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&amp;data=04%7C01%7Cudo%40vmware.com%7Ce4453349214a4092820c08d86b146981%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637377082704851012%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=YWHUpuRLR4xAxNpJGXQcj1J1A1Cm2IEWO1G6I45uFiE%3D&amp;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&amp;data=04%7C01%7Cudo%40vmware.com%7Ce4453349214a4092820c08d86b146981%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637377082704851012%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=692vGLzF%2Bbadx36kFLWR996pjtlkE9otE4Rr8z1UVHA%3D&amp;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
        > 
        > 
        > 



Reply via email to