You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cayenne.apache.org by Aristedes Maniatis <ar...@ish.com.au.INVALID> on 2021/10/10 07:04:19 UTC
cgen into the future
On 10/10/21 5:42pm, Andrus Adamchik wrote:
> A bit OT... Before 4.1 all my projects used cgen and most - cdbimport. Not anymore. Things went full circle and Modeler has become the only way to sync up project with the DB. DataMap XML file now stores all the settings of the c* tasks. So now I came to realization that putting those same settings in your build file (Ant/Maven/Gradle) was simply a hack that allowed to share and version-control task configs. It is not needed anymore.
>
> While I'll expect pushback on this:) , may I suggest to deprecate the tools in 5.0, and remove them in 5.1.
Can you explain a bit more? We don't store generated superclasses in
version control since that's confusing and redundant. [1] This is
similar to how we don't store swagger generated classes in version
control for the server side API implementation.
Ari
[1] https://github.com/ishgroup/oncourse/blob/main/server/build.gradle#L335
[2]
https://github.com/ishgroup/oncourse/blob/main/server-api/build.gradle#L62
Re: cgen into the future
Posted by Mike Kienenberger <mk...@gmail.com>.
On Sun, Oct 10, 2021 at 5:09 AM Andrus Adamchik <aa...@gmail.com> wrote:
> I expected tools removal to be a controversial proposal. Still decided to
> throw it out there. I am always looking for opportunities to minimize our
> support footprint (hi, ROP :)). Properly maintaining the tools across 3
> build systems is a non-trivial effort (especially Gradle, due to the rest
> of Cayenne being built with Maven). Now we've only increased the footprint,
> as we'd need to be merging map.xml config with build file config. But of
> course I realize that lots of people have designed their Cayenne workflow
> around the build tools (and we encouraged it), and will not appreciate them
> being taken away.
>
I use cgen for more than just generating entity superclasses and
subclasses. I use it for generating DAO objects, presentation layer
components, validation files, and even entire presentation layer pages in
some cases. There is at least one internal CRUD app which is 99%
generated from cgen with very little original code.
Re: cgen into the future
Posted by Andrus Adamchik <aa...@gmail.com>.
> I don't see the benefit in storing the output of generated code in version control.
There's no benefit, but there's no downside either. I barely give it a thought during my daily work. Entity removal is probably the only snag, but it is not completely addressed by not having superclasses in VC: you still have to remove you subclass manually.
> With the configuration in our gradle build file there is absolutely nothing to ask new developers to do.
Yep. This is why it was recommended by us before it got moved to the Modeler. Now the "Modeler-only" flow is at the same (or arguably higher) level of convenience.
> But I'd be equally happy for Modeler and cgen to share the same configuration somewhere.
I don't remember if they can already share the config from the map.xml file. But they definitely should.
I expected tools removal to be a controversial proposal. Still decided to throw it out there. I am always looking for opportunities to minimize our support footprint (hi, ROP :)). Properly maintaining the tools across 3 build systems is a non-trivial effort (especially Gradle, due to the rest of Cayenne being built with Maven). Now we've only increased the footprint, as we'd need to be merging map.xml config with build file config. But of course I realize that lots of people have designed their Cayenne workflow around the build tools (and we encouraged it), and will not appreciate them being taken away.
FWIW, the switch was very simple for me and my team, but that may be because "cdbimport" was also in use, and going from pom.xml to the Modeler for cdbimport removed so much pain, that it was a no-brainer. "cgen" was just icing on the cake. We are using standard templates and nobody cared about the latter one way or another (and we already had _Xyz.java under version control).
Andrus
> On Oct 10, 2021, at 11:41 AM, Aristedes Maniatis <ar...@ish.com.au.INVALID> wrote:
>
> I don't see the benefit in storing the output of generated code in version control. It makes commit history more noisy and hard to read without any real benefit. For just this one project, that includes:
>
> * Cayenne
> * Swagger
> * Antlr
>
> So if someone makes a change to the data model of one of those things and forgets to commit additional files, or there is a lot of boilerplate output changes because (say) Swagger was updated, then things might get out of sync and cause oddness in builds.
>
> Mostly importantly, its really easy to mess up when you remove an entity from Cayenne and there is no automated process which also removes the relevant superclass. You may not even notice for a while until something unexpected happens three months later.
>
> With the configuration in our gradle build file there is absolutely nothing to ask new developers to do. But I'd be equally happy for Modeler and cgen to share the same configuration somewhere.
>
>
> Ari
>
>
> On 10/10/21 6:25pm, Andrus Adamchik wrote:
>>> We don't store generated superclasses in version control since that's confusing and redundant.
>> I commit generated superclasses to Git everywhere. Never caused any issues that I can remember. But yes, I think that would be a prerequisite to switching away from cgen in build scripts to the Modeler-based flow.
>>
>> What I was alluding to is that the XML config below is now managed visually in the Modeler and is saved in the XYZ.map.xml, while previously it was only saved in the local modeler preferences. So keeping a similar configuration in a build script was a superior approach because it was the only way to keep the config portable across dev machines. Imagine asking all your team members to manually import a custom template and setup their Modeler to use it for a specific project. Now this problem is gone.
>>
>>
>> <dbImport xmlns="http://cayenne.apache.org/schema/10/dbimport">
>> <catalog>
>> <includeTable>
>> <name>ma_.*</name>
>> </includeTable>
>> <name>media</name>
>> </catalog>
>> <tableTypes>
>> <tableType>TABLE</tableType>
>> <tableType>VIEW</tableType>
>> </tableTypes>
>> <forceDataMapCatalog>false</forceDataMapCatalog>
>> <forceDataMapSchema>false</forceDataMapSchema>
>> <namingStrategy>org.apache.cayenne.dbsync.naming.DefaultObjectNameGenerator</namingStrategy>
>> <skipPrimaryKeyLoading>false</skipPrimaryKeyLoading>
>> <skipRelationshipsLoading>false</skipRelationshipsLoading>
>> <stripFromTableNames>^ma_</stripFromTableNames>
>> <useJava7Types>false</useJava7Types>
>> <usePrimitives>true</usePrimitives>
>> </dbImport>
>> <cgen xmlns="http://cayenne.apache.org/schema/10/cgen">
>> <destDir>../../../../../../java</destDir>
>> <mode>entity</mode>
>> <template>templates/v4_1/subclass.vm</template>
>> <superTemplate>templates/v4_1/superclass.vm</superTemplate>
>> <outputPattern>*.java</outputPattern>
>> <makePairs>true</makePairs>
>> <usePkgPath>true</usePkgPath>
>> <overwrite>false</overwrite>
>> <createPropertyNames>false</createPropertyNames>
>> <createPKProperties>false</createPKProperties>
>> <client>false</client>
>> </cgen>
>>
>> Andrus
>>
>>
>>> On Oct 10, 2021, at 10:04 AM, Aristedes Maniatis <ar...@ish.com.au.INVALID> wrote:
>>>
>>>
>>>
>>> On 10/10/21 5:42pm, Andrus Adamchik wrote:
>>>> A bit OT... Before 4.1 all my projects used cgen and most - cdbimport. Not anymore. Things went full circle and Modeler has become the only way to sync up project with the DB. DataMap XML file now stores all the settings of the c* tasks. So now I came to realization that putting those same settings in your build file (Ant/Maven/Gradle) was simply a hack that allowed to share and version-control task configs. It is not needed anymore.
>>>>
>>>> While I'll expect pushback on this:) , may I suggest to deprecate the tools in 5.0, and remove them in 5.1.
>>> Can you explain a bit more? We don't store generated superclasses in version control since that's confusing and redundant. [1] This is similar to how we don't store swagger generated classes in version control for the server side API implementation.
>>>
>>>
>>> Ari
>>>
>>>
>>> [1] https://github.com/ishgroup/oncourse/blob/main/server/build.gradle#L335
>>> [2] https://github.com/ishgroup/oncourse/blob/main/server-api/build.gradle#L62
>
Re: cgen into the future
Posted by Aristedes Maniatis <ar...@ish.com.au.INVALID>.
I don't see the benefit in storing the output of generated code in
version control. It makes commit history more noisy and hard to read
without any real benefit. For just this one project, that includes:
* Cayenne
* Swagger
* Antlr
So if someone makes a change to the data model of one of those things
and forgets to commit additional files, or there is a lot of boilerplate
output changes because (say) Swagger was updated, then things might get
out of sync and cause oddness in builds.
Mostly importantly, its really easy to mess up when you remove an entity
from Cayenne and there is no automated process which also removes the
relevant superclass. You may not even notice for a while until something
unexpected happens three months later.
With the configuration in our gradle build file there is absolutely
nothing to ask new developers to do. But I'd be equally happy for
Modeler and cgen to share the same configuration somewhere.
Ari
On 10/10/21 6:25pm, Andrus Adamchik wrote:
>> We don't store generated superclasses in version control since that's confusing and redundant.
> I commit generated superclasses to Git everywhere. Never caused any issues that I can remember. But yes, I think that would be a prerequisite to switching away from cgen in build scripts to the Modeler-based flow.
>
> What I was alluding to is that the XML config below is now managed visually in the Modeler and is saved in the XYZ.map.xml, while previously it was only saved in the local modeler preferences. So keeping a similar configuration in a build script was a superior approach because it was the only way to keep the config portable across dev machines. Imagine asking all your team members to manually import a custom template and setup their Modeler to use it for a specific project. Now this problem is gone.
>
>
> <dbImport xmlns="http://cayenne.apache.org/schema/10/dbimport">
> <catalog>
> <includeTable>
> <name>ma_.*</name>
> </includeTable>
> <name>media</name>
> </catalog>
> <tableTypes>
> <tableType>TABLE</tableType>
> <tableType>VIEW</tableType>
> </tableTypes>
> <forceDataMapCatalog>false</forceDataMapCatalog>
> <forceDataMapSchema>false</forceDataMapSchema>
> <namingStrategy>org.apache.cayenne.dbsync.naming.DefaultObjectNameGenerator</namingStrategy>
> <skipPrimaryKeyLoading>false</skipPrimaryKeyLoading>
> <skipRelationshipsLoading>false</skipRelationshipsLoading>
> <stripFromTableNames>^ma_</stripFromTableNames>
> <useJava7Types>false</useJava7Types>
> <usePrimitives>true</usePrimitives>
> </dbImport>
> <cgen xmlns="http://cayenne.apache.org/schema/10/cgen">
> <destDir>../../../../../../java</destDir>
> <mode>entity</mode>
> <template>templates/v4_1/subclass.vm</template>
> <superTemplate>templates/v4_1/superclass.vm</superTemplate>
> <outputPattern>*.java</outputPattern>
> <makePairs>true</makePairs>
> <usePkgPath>true</usePkgPath>
> <overwrite>false</overwrite>
> <createPropertyNames>false</createPropertyNames>
> <createPKProperties>false</createPKProperties>
> <client>false</client>
> </cgen>
>
> Andrus
>
>
>> On Oct 10, 2021, at 10:04 AM, Aristedes Maniatis <ar...@ish.com.au.INVALID> wrote:
>>
>>
>>
>> On 10/10/21 5:42pm, Andrus Adamchik wrote:
>>> A bit OT... Before 4.1 all my projects used cgen and most - cdbimport. Not anymore. Things went full circle and Modeler has become the only way to sync up project with the DB. DataMap XML file now stores all the settings of the c* tasks. So now I came to realization that putting those same settings in your build file (Ant/Maven/Gradle) was simply a hack that allowed to share and version-control task configs. It is not needed anymore.
>>>
>>> While I'll expect pushback on this:) , may I suggest to deprecate the tools in 5.0, and remove them in 5.1.
>> Can you explain a bit more? We don't store generated superclasses in version control since that's confusing and redundant. [1] This is similar to how we don't store swagger generated classes in version control for the server side API implementation.
>>
>>
>> Ari
>>
>>
>> [1] https://github.com/ishgroup/oncourse/blob/main/server/build.gradle#L335
>> [2] https://github.com/ishgroup/oncourse/blob/main/server-api/build.gradle#L62
Re: cgen into the future
Posted by John Huss <jo...@gmail.com>.
Besides generating the java classes, we use cgen to generate source code
for other languages and other artifacts, using different configuration. So
while I would be in favor of removing the duplication of the configuration
for java class generation, I would still like to have the cgen task remain
available to use for other purposes.
On Sun, Oct 10, 2021 at 2:26 AM Andrus Adamchik <aa...@gmail.com> wrote:
> > We don't store generated superclasses in version control since that's
> confusing and redundant.
>
> I commit generated superclasses to Git everywhere. Never caused any issues
> that I can remember. But yes, I think that would be a prerequisite to
> switching away from cgen in build scripts to the Modeler-based flow.
>
> What I was alluding to is that the XML config below is now managed
> visually in the Modeler and is saved in the XYZ.map.xml, while previously
> it was only saved in the local modeler preferences. So keeping a similar
> configuration in a build script was a superior approach because it was the
> only way to keep the config portable across dev machines. Imagine asking
> all your team members to manually import a custom template and setup their
> Modeler to use it for a specific project. Now this problem is gone.
>
>
> <dbImport xmlns="http://cayenne.apache.org/schema/10/dbimport">
> <catalog>
> <includeTable>
> <name>ma_.*</name>
> </includeTable>
> <name>media</name>
> </catalog>
> <tableTypes>
> <tableType>TABLE</tableType>
> <tableType>VIEW</tableType>
> </tableTypes>
> <forceDataMapCatalog>false</forceDataMapCatalog>
> <forceDataMapSchema>false</forceDataMapSchema>
>
> <namingStrategy>org.apache.cayenne.dbsync.naming.DefaultObjectNameGenerator</namingStrategy>
> <skipPrimaryKeyLoading>false</skipPrimaryKeyLoading>
> <skipRelationshipsLoading>false</skipRelationshipsLoading>
> <stripFromTableNames>^ma_</stripFromTableNames>
> <useJava7Types>false</useJava7Types>
> <usePrimitives>true</usePrimitives>
> </dbImport>
> <cgen xmlns="http://cayenne.apache.org/schema/10/cgen">
> <destDir>../../../../../../java</destDir>
> <mode>entity</mode>
> <template>templates/v4_1/subclass.vm</template>
> <superTemplate>templates/v4_1/superclass.vm</superTemplate>
> <outputPattern>*.java</outputPattern>
> <makePairs>true</makePairs>
> <usePkgPath>true</usePkgPath>
> <overwrite>false</overwrite>
> <createPropertyNames>false</createPropertyNames>
> <createPKProperties>false</createPKProperties>
> <client>false</client>
> </cgen>
>
> Andrus
>
>
> > On Oct 10, 2021, at 10:04 AM, Aristedes Maniatis <ar...@ish.com.au.INVALID>
> wrote:
> >
> >
> >
> > On 10/10/21 5:42pm, Andrus Adamchik wrote:
> >> A bit OT... Before 4.1 all my projects used cgen and most - cdbimport.
> Not anymore. Things went full circle and Modeler has become the only way to
> sync up project with the DB. DataMap XML file now stores all the settings
> of the c* tasks. So now I came to realization that putting those same
> settings in your build file (Ant/Maven/Gradle) was simply a hack that
> allowed to share and version-control task configs. It is not needed anymore.
> >>
> >> While I'll expect pushback on this:) , may I suggest to deprecate the
> tools in 5.0, and remove them in 5.1.
> >
> > Can you explain a bit more? We don't store generated superclasses in
> version control since that's confusing and redundant. [1] This is similar
> to how we don't store swagger generated classes in version control for the
> server side API implementation.
> >
> >
> > Ari
> >
> >
> > [1]
> https://github.com/ishgroup/oncourse/blob/main/server/build.gradle#L335
> > [2]
> https://github.com/ishgroup/oncourse/blob/main/server-api/build.gradle#L62
>
>
Re: cgen into the future
Posted by Andrus Adamchik <aa...@gmail.com>.
> We don't store generated superclasses in version control since that's confusing and redundant.
I commit generated superclasses to Git everywhere. Never caused any issues that I can remember. But yes, I think that would be a prerequisite to switching away from cgen in build scripts to the Modeler-based flow.
What I was alluding to is that the XML config below is now managed visually in the Modeler and is saved in the XYZ.map.xml, while previously it was only saved in the local modeler preferences. So keeping a similar configuration in a build script was a superior approach because it was the only way to keep the config portable across dev machines. Imagine asking all your team members to manually import a custom template and setup their Modeler to use it for a specific project. Now this problem is gone.
<dbImport xmlns="http://cayenne.apache.org/schema/10/dbimport">
<catalog>
<includeTable>
<name>ma_.*</name>
</includeTable>
<name>media</name>
</catalog>
<tableTypes>
<tableType>TABLE</tableType>
<tableType>VIEW</tableType>
</tableTypes>
<forceDataMapCatalog>false</forceDataMapCatalog>
<forceDataMapSchema>false</forceDataMapSchema>
<namingStrategy>org.apache.cayenne.dbsync.naming.DefaultObjectNameGenerator</namingStrategy>
<skipPrimaryKeyLoading>false</skipPrimaryKeyLoading>
<skipRelationshipsLoading>false</skipRelationshipsLoading>
<stripFromTableNames>^ma_</stripFromTableNames>
<useJava7Types>false</useJava7Types>
<usePrimitives>true</usePrimitives>
</dbImport>
<cgen xmlns="http://cayenne.apache.org/schema/10/cgen">
<destDir>../../../../../../java</destDir>
<mode>entity</mode>
<template>templates/v4_1/subclass.vm</template>
<superTemplate>templates/v4_1/superclass.vm</superTemplate>
<outputPattern>*.java</outputPattern>
<makePairs>true</makePairs>
<usePkgPath>true</usePkgPath>
<overwrite>false</overwrite>
<createPropertyNames>false</createPropertyNames>
<createPKProperties>false</createPKProperties>
<client>false</client>
</cgen>
Andrus
> On Oct 10, 2021, at 10:04 AM, Aristedes Maniatis <ar...@ish.com.au.INVALID> wrote:
>
>
>
> On 10/10/21 5:42pm, Andrus Adamchik wrote:
>> A bit OT... Before 4.1 all my projects used cgen and most - cdbimport. Not anymore. Things went full circle and Modeler has become the only way to sync up project with the DB. DataMap XML file now stores all the settings of the c* tasks. So now I came to realization that putting those same settings in your build file (Ant/Maven/Gradle) was simply a hack that allowed to share and version-control task configs. It is not needed anymore.
>>
>> While I'll expect pushback on this:) , may I suggest to deprecate the tools in 5.0, and remove them in 5.1.
>
> Can you explain a bit more? We don't store generated superclasses in version control since that's confusing and redundant. [1] This is similar to how we don't store swagger generated classes in version control for the server side API implementation.
>
>
> Ari
>
>
> [1] https://github.com/ishgroup/oncourse/blob/main/server/build.gradle#L335
> [2] https://github.com/ishgroup/oncourse/blob/main/server-api/build.gradle#L62
Re: cgen into the future
Posted by Aristedes Maniatis <ar...@ish.com.au.INVALID>.
On 11/10/21 2:56am, Michael Gentry wrote:
> If someone generated the "incorrect" classes
> from the default templates, cgen would fix that mistake for them when they
> built the project (happened frequently enough that I made it part of the
> build process).
This isn't true when deleting or renaming an entity in the model. It is
really important to me that this kind of mistake causes a compilation
error. Multiply this problem out by antlr, swagger and cayenne entities
and I'm very happy with our design choice to keep generated code out of VCS.
This isn't hypothetical. Years ago we had a real problem that took days
to solve; additional entities in our classpath caused a subtle problem
in places you'd not expect. Almost lost a customer over it, so I've
little interest in going back there.
> if you
> have to checkout a previous version and build it with a newer (or perhaps
> even older) version of Cayenne to see the superclasses, you may not be
> seeing the same superclasses as originally intended.
I'm pretty happy with our use of gradle to lock dependencies to a
reproducible version. We even use gradle to lock in the version of node
which compiles our typescript. And our DSL documentation is generated
live [1], parsing the code without having to remember to commit the
generated docs.
Anyhow, each to their own. Some people might like to commit js files
compiled from typescript so they can search on what is running in
production without compiling. For us, the opposite approach has worked well.
Cheers
Ari
[1]
https://github.com/ishgroup/oncourse/blob/main/buildSrc/apidoc/src/main/groovy/au/com/ish/docs/DslGroovyRootDocBuilder.groovy
Re: cgen into the future
Posted by Michael Gentry <bl...@gmail.com>.
On Sun, Oct 10, 2021 at 3:04 AM Aristedes Maniatis <ar...@ish.com.au.invalid>
wrote:
> Can you explain a bit more? We don't store generated superclasses in
> version control since that's confusing and redundant. [1] This is
> similar to how we don't store swagger generated classes in version
> control for the server side API implementation.
>
Hi Ari and Andrus,
While I'm generally a proponent of "don't commit generated artifacts," I'm
also a proponent of "write code to be read, not written."
I often browse code through GitHub or BitBucket and if I were browsing a
Cayenne project WITHOUT the superclasses checked in, I'd probably be
grumbling "WTF is the code?!?!?!" [1] There are also many times I've sent
people a GH or BB link to code to look at, including the superclasses. I
wouldn't want to have to checkout and build before I could browse the
code, which could involve a lot of steps for some of the projects I've seen
or worked on. Likewise, I wouldn't want to tell someone else they have
to checkout and build the code to see the superclasses.
Also, with the superclass checked in, you have a point-in-time view of the
actual code which was used to build the project. With changes to Cayenne
Modeler and code generation possible from one version to another, if you
have to checkout a previous version and build it with a newer (or perhaps
even older) version of Cayenne to see the superclasses, you may not be
seeing the same superclasses as originally intended.
I guess I feel it is important to check in the generated classes for
readability and historical purposes. Also used cgen quite a bit to generate
the classes instead of using Cayenne Modeler to do so since it made dealing
with custom templates easier. If someone generated the "incorrect" classes
from the default templates, cgen would fix that mistake for them when they
built the project (happened frequently enough that I made it part of the
build process).
[1] This might be even more important if you use custom templates for a
project.
Thanks,
mrg