odpi / egeria-database-connectors

Connectors for exchanging metadata
Apache License 2.0
16 stars 10 forks source link

Build connector jar without egeria 'platform' dependencies #12

Closed planetf1 closed 3 years ago

planetf1 commented 3 years ago

When the postgres connector is built we are creating an uber jar (jar with dependencies). This pulls in all dependencies in 'implementation' scope, which includes the key egeria artifacts needed

However when this is loaded at runtime into the egeria server chassis, a few of those may be duplicates of classes already found in the chassis. If versions differ this could lead to initialization or class loading issues.

This is discussed further in https://github.com/odpi/egeria/issues/4455 where clarification over exactly what the platform is, and guidelines for connector writers, should be added.

However by inspection we see that the current packages requried by the postgres connector are:

     'org.odpi.egeria:data-manager-client'
     'org.odpi.egeria:data-manager-api'
     'org.odpi.egeria:database-integrator-api'
     'org.odpi.egeria:open-connector-framework'

We can see the packages in the current server chassis by 'mvn dependency:tree' in the open-metadata-implementation/server-chassis/server-chassis-spring directory

Every single of these packages is already in the server chassis

Thus at this point in time it would be safer to change these dependencies to:

     compileOnly 'org.odpi.egeria:data-manager-client'
     compileOnly 'org.odpi.egeria:data-manager-api'
     compileOnly 'org.odpi.egeria:database-integrator-api'
     compileOnly 'org.odpi.egeria:open-connector-framework'

Note the use of 'compileOnly' which is basically the same as 'provided' scope in maven. This way the classes are used for compilation, but are not built into the uber jar -- there are other ways of doing this given the explicit nature of gradle, but this native approach seems to be the most standard.

The challenge for connector developers is getting clarity on what they deem as provided, and what isn't.

One simple option would be everything should be provided (even to the server - so including clients) by default - that can be reviewed further in https://github.com/odpi/egeria/issues/4455

here I propose we make this change to allow the postgres connector to work smoothly now

davidradl commented 3 years ago

I agree - this looks like a good change. Where to we get the version we compile against? Is this master or the last released version. What can we do to try to avoid compiling against one version and the runtime has an incompatible version of the dependancy.

planetf1 commented 3 years ago

@davidradl Currently the connector (as in 'main') is building against 2.6-SNAPSHOT, and has an additional (to Maven Central/JCenter) repository defined for our artifactory server where all merge builds are stored. So basically it's always building against the latest 2.6

The rationale for this is that we are currently very much still in development, as the Integration patterns evolve.

Once that code becomes stable (maybe that's 2.7) it would make sense to instead build connectors against the last known good release. Then we can also remove the definition of the snapshot repo in artifactory

A similar pattern was adopted for the igc/atlas connectors. Not exactly defined, but initially they were part of the code build & so always built with the latest code. Then they migrated to their own repos and now tend to depend on the last release rather than any snapshot. This is also more representative of 3rd party connectors (though anyone can access our merge build snapshots if they wish - it's public)

planetf1 commented 3 years ago

The current connector build no longer includes unnecessary dependencies, so closing the issue here Further work will be done in base egeria on the included connectors to adopt a more streamlined approach