> > Could you add a README file at the root of the `persistence/` directory so > that it is clear that eclipselink is an exception to the new structure?
Good point! As an alternative, we could also explain it in a README of the EclipseLink module. Let's do that in a follow-up. we have "api", "common" and > potentially other modules under persistence. The current persistence APIs and common interfaces are in the core module. I'd like to keep it as is. For the future common or API classes, we could still put them there as well. Besides, per this doc https://docs.google.com/document/d/1FEQ3f1XXKG_H7QFI-LN8lEkVljXoNNl2Bx4HVmj3UEI/edit?tab=t.0#heading=h.b7qgf1q8h0w, it's critical to not leak any persistence implementation related things to other modules. Let's evaluate them separately. Thanks everyone for the feedback. Great that we got a consensus on PR 1724. I will merge it. We can continue to discuss related topics here, like renaming "relational-jdbc" or moving to a new thread. Yufei On Mon, Jun 2, 2025 at 9:26 AM Dmitri Bourlatchkov <dmitri.bourlatch...@dremio.com.invalid> wrote: > After the refactor, there is a top level directory named "persistence", > which holds two implementations -- EclipseLink and JDBC. I think it makes > sense to hold more persistence impl. in the future in parallel with them > like the following example shows. WDYT? > > > I agree with that proposal. > > However, at to this specific dir structure under /persistence: > > persistence/ > ├── eclipselink/ > ├── relational-jdbc/ > ├── mongodb/ > └── .../ > > > ... I do not think it is flexible enough. I'm sure there's going to be a > need to share code across different implementations. > > I'm in favour of JB's proposal, where we have "api", "common" and > potentially other modules under persistence. > > As for `persistence/mongodb` - this name LGTM. However, I'm pretty sure the > end-to-end NoSQL persistence impl. for Mongo will require more modules > directly under `/persistence`. > > Side note on `relational-jdbc`: I do not think current JDBC Persistence is > "relational" - it does not actualize entity relationships at the database > level. I believe "transactional" is a more relevant term (since it does > rely on updating multiple rows in the same JDBC Tx). I do not insist on > renaming this module, but if other people agree, this renaming might be > worth doing. > > Cheers, > Dmitri. > > On Sat, May 31, 2025 at 7:34 PM Yufei Gu <flyrain...@gmail.com> wrote: > > > Dmitri, thanks for the feedback. > > > > The PR focus on removing the layer of directories("extensions"). It is > > unnecessary and confusing. I'm glad we got a consensus here. > > > > After the refactor, there is a top level directory named "persistence", > > which holds two implementations -- EclipseLink and JDBC. I think it makes > > sense to hold more persistence impl. in the future in parallel with them > > like the following example shows. WDYT? > > > > persistence/ > > ├── eclipselink/ > > ├── relational-jdbc/ > > ├── mongodb/ > > └── .../ > > > > Yufei > > > > > > On Fri, May 30, 2025 at 5:16 PM Dmitri Bourlatchkov <di...@apache.org> > > wrote: > > > > > Hi Yufei, > > > > > > Thanks for opening a dev list discussion for this. > > > > > > Re: PR #1724 - moving "persistence" from the "extensions" sub-dir to > the > > > project root level, LGTM. > > > > > > However, my comment in GH about dev list discussions was more general. > > > Since we start moving things around in one sub-tree, I'd like to > clarify > > > what target repository layout is envisioned across all modules. > > > > > > Do you have any similar moves in mind or is this change limited only to > > the > > > "extensions" directory? > > > > > > I suppose now may be a good time to have that discussion. > > > Specifically having in mind the proposed NoSQL persistence (which is > not > > > merged yet). > > > > > > Thanks, > > > Dmitri. > > > > > > On Thu, May 29, 2025 at 5:00 PM Yufei Gu <flyrain...@gmail.com> wrote: > > > > > > > Hi everyone, > > > > > > > > I’d like to draw attention to PR #1724 ( > > > > https://github.com/apache/polaris/pull/1724) which reorganises our > > > > persistence modules: > > > > > > > > - > > > > > > > > Moves *EclipseLink* and *JDBC* from extension/persistence/impl/* > to > > a > > > > top-level persistence/* directory. > > > > - > > > > > > > > Aligns most Java packages from > > > > org.apache.polaris.extension.persistence.impl.* → > > > > org.apache.polaris.persistence.impl.*. > > > > - > > > > > > > > *Note:* EclipseLink keeps its original package to avoid > breaking > > > > external integrations and because we plan to deprecate/remove > > > > the module in > > > > a future release. > > > > - > > > > > > > > *No behavioural changes* – the PR is strictly a mechanical > > > move/rename. > > > > > > > > *Why?* > > > > This cleans up the repo structure ahead of 1.0, making it clearer > where > > > > first-class vs. extension modules live, and reduces depth in package > > > names. > > > > > > > > I’d like to get agreement on this new layout before we merge. Please > > > reply > > > > with any concerns or +1s. If there are no objections within 72 hours, > > > we’ll > > > > proceed. > > > > > > > > Thanks, > > > > Yufei > > > > > > > > > >
