Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PIP-200: Package Pulsar Trino distro and config in a dedicated folder #17137

Closed
1 task done
tisonkun opened this issue Aug 17, 2022 · 1 comment
Closed
1 task done
Labels

Comments

@tisonkun
Copy link
Member

tisonkun commented Aug 17, 2022

Mailing list thread: https://lists.apache.org/thread/s985ypf0r0hzcm0mx653n5h2rt7n273v

Motivation

After #16683 merged, we upgrade PrestoSQL dependency in Pulsar SQL to the first several Trino version. To handle the name change cases and gradually refactor Pulsar SQL as a self-contained module so that we can move it into a standalone repository, I find that there're three major issues to resolve.

  1. Configs of Pulsar SQL go under the conf/ folder and mix with other Pulsar configs.
  2. Pulsar Docker images (base and all) bundle Pulsar SQL.
  3. Integration tests of Pulsar SQL are tightly coupled with the main repo (test infra).

This proposal focuses on the first step only.

It's to package Pulsar Trino distro and config in a dedicated folder; that is, to make the distro self-contained.

Goal

I have already prepared a draft to perform the changes as #17062. Generally, we move the config files under PRESTO_HOME and correspondingly update scripts.

In this way, all Trino distro artifacts are under the same home path, so that we can later move it out as a whole.

Compatibility

This change should not affect those who use Pulsar with the entry point script, but it changes the layout of the release artifact.

I'm going to write a release note about this change and also post it on the Pulsar SQL overview page as a caveat. Draft here:

# Caveat

If you're upgrading Pulsar SQL from 2.11 or early, you should copy the related configs from `conf/presto` to `trino/conf`, and `lib/presto` to `trino`. If you're downgrading Pulsar SQL to 2.11 or early from 2.12, do verse visa.

Implementation

It's straightforward to inline in the "Goal" section.

I will move all artifacts now under conf/presto and lib/presto to a top-level folder trino, while keeping all conf files under trino/conf. Besides, all relevant references in scripts will be updated.

To minimize unnecessary changes, I tend to keep the module's name pulsar-presto-xxx as is.

Alternatives

I don't make a completed proposal to resolve all three issues listed above. Because I'm still unfamiliar with the latter two topics yet and I'd prefer to implement these improvements one by one since they're naturally independent. If I try to make a completed proposal at once, it's highly possible I give up halfway.

Anything else?

Previous discussion:

@tisonkun
Copy link
Member Author

Closed as all subtasks done.

cc @codelipenghui @eolivelli perhaps we should note somewhere that this change need a release note. The ceveat is included in 33905b6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant