The purpose of code generation from the openapi spec is to ensure that api changes are made spec-first. There is no opportunity to forget to change the spec and there’s no worry about the code and the spec deviating (at least, the endpoints don’t differ). The generated code, if “not great” is extremely limited in scope and application. It’s not pervasive throughout the codebase. It doesn’t require some deep knowledge to understand or debug. And its correctness has no impact on the execution of any of the rest of the code in the project. Comparing this to bytecode or compiler agents is apples to oranges
Mike On Mon, Feb 17, 2025 at 7:47 AM Eric Maynard <eric.w.mayn...@gmail.com> wrote: > Generating code based on the spec is completely different than generating > code based on the annotations introduced by Immutables. > > More importantly, we shouldn’t allow a discussion about pushing the > project forward to devolve into a debate around some third-party library > that’s not necessary to the change. > > On Mon, Feb 17, 2025 at 9:42 AM Robert Stupp <sn...@snazy.de> wrote: > >> Everything you have to code manually requires explicit tests. >> >> Immutables is a very mature library that has exceptionally good test >> coverage that prevents a ton of accidental coding mistakes. We've been >> using it for many many years in multiple projects and literally never >> produced wrong code. >> >> We already have code generators in the code base for openapi. I'd okay >> to remove that, because the code generated by that generator is IMHO not >> great. >> >> Overall we should focus on the "real stuff" and not bother about >> manually writing builders and really immutable types. >> >> On 17.02.25 18:10, Michael Collado wrote: >> > I prefer native Java constructs over third party libraries and compiler >> > plugins, whenever possible. I’m a fan of Java records. >> > >> > Mike >> > >> > On Mon, Feb 17, 2025 at 6:55 AM Jean-Baptiste Onofré <j...@nanthrax.net> >> > wrote: >> > >> >> Hi Robert, >> >> >> >> Yes, my intention about Java records (e.g. >> >> https://openjdk.org/jeps/395) is to leverage: >> >> - Devise an object-oriented construct that expresses a simple >> >> aggregation of values. >> >> - Focus on modeling immutable data rather than extensible behavior. >> >> - Automatically implement data-driven methods such as equals and >> accessors. >> >> - Preserve long-standing Java principles such as nominal typing and >> >> migration compatibility. >> >> - Provide inner builder, optionally with technical validation >> >> (Objects.NotNull, etc), for instance: >> >> >> >> public record CatalogDAO(String id, String name, ...) { >> >> >> >> public static final class Builder { >> >> String id; >> >> String name; >> >> public Builder() {} >> >> public Builder id(String id) { >> >> this.id = id; >> >> return this; >> >> } >> >> public Builder name(String name) { >> >> this.name = name; >> >> return this; >> >> } >> >> public CatalogDAO build() { >> >> return new CatalogDAO(id, name); >> >> } >> >> } >> >> >> >> } >> >> NB: that's just a "raw" example :) >> >> >> >> That could help us for the "DAO" layer, with clean isolation and data >> >> "transport". The "conversion" from Polaris Entity to DAO could be >> >> performed by the intermediate logic layer (e.g. >> >> PolarisMetaStoreManager), the pure persistence layer can deal with DAO >> >> only (e.g. PolarisStore). >> >> >> >> Good point about immutable collections. In some projects, I "force" >> >> the copy of a collection to ensure immutability, something like: >> >> >> >> record NamespaceDAO(Set<String> children) { >> >> public NamespaceDAO { >> >> children = Set.copyOf(children); >> >> } >> >> } >> >> >> >> OK, that's not super elegant :) but it does the trick ;) >> >> >> >> Regards >> >> JB >> >> >> >> On Mon, Feb 17, 2025 at 5:28 PM Robert Stupp <sn...@snazy.de> wrote: >> >>> Agree with JB, except I think "immutables" serves the need much >> better. >> >>> Java records are okay, but do not ensure that all fields are >> immutable - >> >>> especially collections. The other pro of immutables is that there are >> >>> proper, descriptive builders - hence no need to constructors with a >> >>> gazillion parameters. >> >>> >> >>> On 17.02.25 11:42, Jean-Baptiste Onofré wrote: >> >>>> Hi Yufei >> >>>> >> >>>> I left comments in the PR. >> >>>> >> >>>> Thanks for that. That's an interesting approach but slightly >> different >> >>>> to what I had in mind. >> >>>> >> >>>> My proposal was: >> >>>> >> >>>> 1. The DAOs are Java records clearly descriptive, without "storage >> >> operations". >> >>>> For instance, we can imagine: >> >>>> >> >>>> public record CatalogDao(String id, String name, String location, >> ...) >> >> { >> >>>> ... >> >>>> } >> >>>> >> >>>> public record NamespaceDao(String id, String name, String parent, >> ...) >> >> {} >> >>>> public record TableDao(String id, String name, ...) {} >> >>>> >> >>>> etc >> >>>> >> >>>> The advantages are: >> >>>> - very simple and descriptive >> >>>> - common to any backend implementation >> >>>> >> >>>> 2. The PolarisStore defines the DAO backend operations and mapping to >> >>>> "internal" Polaris entities. Each persistence adapter implements the >> >>>> PolarisStore. >> >>>> For the persistence adapter implementer, he just has to implement the >> >>>> DAO persistence (no need to understand other Polaris parts). >> >>>> Each persistence adapter is in its own module (for isolation and >> >> dependencies). >> >>>> During the Polaris Persistence Meeting last week, we already got >> >>>> consensus on the approach proposed by Dennis and I. I propose to do a >> >>>> new sync/review with Dennis. >> >>>> >> >>>> Regards >> >>>> JB >> >>>> >> >>>> On Mon, Feb 17, 2025 at 9:40 AM Yufei Gu <flyrain...@gmail.com> >> wrote: >> >>>>> Hi Folks: >> >>>>> >> >>>>> Here is a POC for persistence layer refactor. Please check it out >> and >> >> let >> >>>>> me know what you think. Please note this is a POC, we still need a >> >> lot of >> >>>>> effort to complete the refactor. >> >>>>> >> >>>>> PR: https://github.com/apache/polaris/pull/1011. >> >>>>> Design doc: >> >>>>> >> >> >> https://docs.google.com/document/d/1Vuhw5b9-6KAol2vU3HUs9FJwcgWtiVVXMYhLtGmz53s/edit?tab=t.0 >> >>>>> Experiment: >> >>>>> >> >>>>> - Added a DAO layer for the business entity namespace(except >> the >> >> read). >> >>>>> - Integrated with existing DAO components >> >> (PolarisMetaStoreManager and >> >>>>> PolarisMetaStoreSession). >> >>>>> - All tests passed successfully, including a manual local run >> >> with Spark >> >>>>> sql. >> >>>>> >> >>>>> Benefits: >> >>>>> >> >>>>> - Compatible with the existing backend(FDB), as we hide it >> behind >> >> the >> >>>>> new DAO. >> >>>>> - Adding new backends(Postgres/MongoDB) is much easier now, esp >> >> for >> >>>>> Postgres, we could be able to use a similar model as Iceberg >> Jdbc >> >> catalog. >> >>>>> - Allows gradual refactoring to remove old DAO dependencies >> >>>>> (PolarisMetaStoreManager and PolarisMetaStoreSession). >> >>>>> - Enables parallel development of new backend implementations. >> >>>>> >> >>>>> Next Steps: >> >>>>> >> >>>>> - Define business entities one by one to decouple them from >> FDB. >> >>>>> - Create DAO interfaces for each entity to standardize >> operations >> >> (e.g., >> >>>>> CRUD, ID generation). >> >>>>> - Expand DAO implementations to support additional backends >> over >> >> time. >> >>>>> >> >>>>> >> >>>>> >> >>>>> Yufei >> >>> -- >> >>> Robert Stupp >> >>> @snazy >> >>> >> -- >> Robert Stupp >> @snazy >> >>
