[Native] Session property manager pass configs to bootstrap#25553

Merged

kevintang2022 merged 1 commit into

prestodb:masterfrom

kevintang2022:export-D78415640

Jul 22, 2025

kevintang2022 commented Jul 16, 2025 •

edited

Loading

Contributor

Summary: Currently, session property manager has an unused parameter called properties so that even if a config is set in a file, for example /etc/session-property-providers/native-worker.properties, the configs are not correctly used.

Differential Revision: D78415640

== RELEASE NOTES ==

General Changes
* Fix native session property manager reading plugin configs from file

kevintang2022 requested review from a team as code owners

July 16, 2025 15:28

prestodb-ci added the from:Meta label

facebook-github-bot commented Jul 16, 2025

Collaborator

This pull request was exported from Phabricator. Differential Revision: D78415640

facebook-github-bot added the fb-exported label

kevintang2022 added a commit to kevintang2022/presto that referenced this pull request


          Session property manager pass configs to bootstrap (prestodb#25553)

248128a

Summary:

Currently, session property manager has an unused parameter called `properties` so that even if a config is set in a file, for example `/etc/session-property-providers/native-worker.properties`, the configs are not correctly used.

Differential Revision: D78415640

kevintang2022 force-pushed the export-D78415640 branch from da2d88f to 248128a Compare

July 16, 2025 15:30

facebook-github-bot commented Jul 16, 2025

Collaborator

This pull request was exported from Phabricator. Differential Revision: D78415640

rschlussel commented Jul 16, 2025

Contributor

nice fix! Can you add a test for this?

aditi-pandit changed the title ~~Session property manager pass configs to bootstrap~~ [Native] Session property manager pass configs to bootstrap

facebook-github-bot commented Jul 16, 2025

Collaborator

This pull request was exported from Phabricator. Differential Revision: D78415640

kevintang2022 added a commit to kevintang2022/presto that referenced this pull request


          Session property manager pass configs to bootstrap (prestodb#25553)

fd0a31d

Summary:
Pull Request resolved: prestodb#25553

Currently, session property manager has an unused parameter called `properties` so that even if a config is set in a file, for example `/etc/session-property-providers/native-worker.properties`, the configs are not correctly used.

Differential Revision: D78415640

kevintang2022 force-pushed the export-D78415640 branch from 248128a to fd0a31d Compare

July 16, 2025 21:36

kevintang2022 requested review from elharo, feilong-liu, jaystarshot and vivek-bharathan as code owners

July 16, 2025 21:36

facebook-github-bot commented Jul 16, 2025

Collaborator

This pull request was exported from Phabricator. Differential Revision: D78415640

kevintang2022 added a commit to kevintang2022/presto that referenced this pull request


          Session property manager pass configs to bootstrap (prestodb#25553)

927f0c3

Summary:
Pull Request resolved: prestodb#25553

Currently, session property manager has an unused parameter called `properties` so that even if a config is set in a file, for example `/etc/session-property-providers/native-worker.properties`, the configs are not correctly used.

Differential Revision: D78415640

kevintang2022 force-pushed the export-D78415640 branch from fd0a31d to 927f0c3 Compare

July 16, 2025 21:44

tdcmeehan requested a review from pdabre12

July 17, 2025 00:21

tdcmeehan requested changes

View reviewed changes

tdcmeehan left a comment

Contributor

Question about the tests

presto-tests/src/main/java/com/facebook/presto/tests/DistributedQueryRunner.java Outdated

kevintang2022 added a commit to kevintang2022/presto that referenced this pull request


          Session property manager pass configs to bootstrap (prestodb#25553)

bd5c716

Summary:

Currently, session property manager has an unused parameter called `properties` so that even if a config is set in a file, for example `/etc/session-property-providers/native-worker.properties`, the configs are not correctly used.

Differential Revision: D78415640

kevintang2022 force-pushed the export-D78415640 branch from 927f0c3 to bd5c716 Compare

July 17, 2025 05:26

facebook-github-bot commented Jul 17, 2025

Collaborator

This pull request was exported from Phabricator. Differential Revision: D78415640

kevintang2022 added a commit to kevintang2022/presto that referenced this pull request


          Session property manager pass configs to bootstrap (prestodb#25553)

2ea4d57

Summary:

Currently, session property manager has an unused parameter called `properties` so that even if a config is set in a file, for example `/etc/session-property-providers/native-worker.properties`, the configs are not correctly used.

Differential Revision: D78415640

kevintang2022 force-pushed the export-D78415640 branch from bd5c716 to 2ea4d57 Compare

July 17, 2025 05:40

facebook-github-bot commented Jul 17, 2025

Collaborator

This pull request was exported from Phabricator. Differential Revision: D78415640

kevintang2022 requested a review from tdcmeehan

July 17, 2025 15:04

pdabre12 requested changes

View reviewed changes

pdabre12 left a comment

Contributor

Thanks @kevintang2022 , have some comments.

presto-tests/src/main/java/com/facebook/presto/tests/DistributedQueryRunner.java Outdated

...native-sidecar-plugin/src/test/java/com/facebook/presto/sidecar/TestNativeSidecarPlugin.java Outdated

presto-tests/src/main/java/com/facebook/presto/tests/DistributedQueryRunner.java Outdated

facebook-github-bot commented Jul 21, 2025

Collaborator

@kevintang2022 has imported this pull request. If you are a Meta employee, you can view this in D78415640.

tdcmeehan previously approved these changes

View reviewed changes

...native-sidecar-plugin/src/test/java/com/facebook/presto/sidecar/TestNativeSidecarPlugin.java Outdated

kevintang2022 added a commit to kevintang2022/presto that referenced this pull request


          Session property manager pass configs to bootstrap (prestodb#25553)

11e4175

Summary:

Currently, session property manager has an unused parameter called `properties` so that even if a config is set in a file, for example `/etc/session-property-providers/native-worker.properties`, the configs are not correctly used.

Differential Revision: D78415640

kevintang2022 dismissed tdcmeehan’s stale review via

11e4175

July 22, 2025 15:36

kevintang2022 force-pushed the export-D78415640 branch from da0c6c6 to 11e4175 Compare

July 22, 2025 15:36

facebook-github-bot commented Jul 22, 2025

Collaborator

@kevintang2022 has imported this pull request. If you are a Meta employee, you can view this in D78415640.

kevintang2022 requested review from pdabre12 and tdcmeehan

July 22, 2025 15:37

tdcmeehan previously approved these changes

View reviewed changes

tdcmeehan left a comment

Contributor

Thanks for fixing this!

pdabre12 reviewed

View reviewed changes

pdabre12 left a comment

Contributor

@kevintang2022 Just one comment

...native-sidecar-plugin/src/test/java/com/facebook/presto/sidecar/TestNativeSidecarPlugin.java


          Session property manager pass configs to bootstrap (prestodb#25553)

4115aa1

Summary:

Currently, session property manager has an unused parameter called `properties` so that even if a config is set in a file, for example `/etc/session-property-providers/native-worker.properties`, the configs are not correctly used.

Differential Revision: D78415640

kevintang2022 dismissed tdcmeehan’s stale review via

4115aa1

July 22, 2025 16:56

kevintang2022 force-pushed the export-D78415640 branch from 11e4175 to 4115aa1 Compare

July 22, 2025 16:56

facebook-github-bot commented Jul 22, 2025

Collaborator

@kevintang2022 has imported this pull request. If you are a Meta employee, you can view this in D78415640.

kevintang2022 requested a review from pdabre12

July 22, 2025 17:07

pdabre12 approved these changes

View reviewed changes

pdabre12 left a comment

Contributor

Thanks @kevintang2022

rschlussel approved these changes

View reviewed changes

amitkdutta approved these changes

View reviewed changes

amitkdutta left a comment

Contributor

Thanks @kevintang2022

kevintang2022 merged commit a31cccf into prestodb:master

110 checks passed

This was referenced Jul 24, 2025

Add release notes for 0.294 unix280/presto#39

Merged

Add release notes for 0.294 unix280/presto#40

Merged

prestodb-ci mentioned this pull request

Add release notes for 0.294 #25633

Merged

6 tasks

pdabre12 mentioned this pull request

[native] Support passing down configs to NativeTypeManager #25894

Open

lga-zurich pushed a commit to lga-zurich/presto-exchange that referenced this pull request


          [Native] Session property manager pass configs to bootstrap (prestodb…

bcac3c0

…#25553)

Summary: Currently, session property manager has an unused parameter
called `properties` so that even if a config is set in a file, for
example `/etc/session-property-providers/native-worker.properties`, the
configs are not correctly used.

Differential Revision: D78415640

```
== RELEASE NOTES ==

General Changes
* Fix native session property manager reading plugin configs from file

lga-zurich pushed a commit to lga-zurich/presto-exchange that referenced this pull request


          [Native] Session property manager pass configs to bootstrap (prestodb…

ef88105

…#25553)

Summary: Currently, session property manager has an unused parameter
called `properties` so that even if a config is set in a file, for
example `/etc/session-property-providers/native-worker.properties`, the
configs are not correctly used.

Differential Revision: D78415640

```
== RELEASE NOTES ==

General Changes
* Fix native session property manager reading plugin configs from file

lga-zurich pushed a commit to lga-zurich/presto-exchange that referenced this pull request


          [Native] Session property manager pass configs to bootstrap (prestodb…

e7468c6

…#25553)

Summary: Currently, session property manager has an unused parameter
called `properties` so that even if a config is set in a file, for
example `/etc/session-property-providers/native-worker.properties`, the
configs are not correctly used.

Differential Revision: D78415640

```
== RELEASE NOTES ==

General Changes
* Fix native session property manager reading plugin configs from file

lga-zurich pushed a commit to lga-zurich/presto-exchange that referenced this pull request


          [Native] Session property manager pass configs to bootstrap (prestodb…

ea4fa66

…#25553)

Summary: Currently, session property manager has an unused parameter
called `properties` so that even if a config is set in a file, for
example `/etc/session-property-providers/native-worker.properties`, the
configs are not correctly used.

Differential Revision: D78415640

```
== RELEASE NOTES ==

General Changes
* Fix native session property manager reading plugin configs from file

lga-zurich pushed a commit to lga-zurich/presto-exchange that referenced this pull request


          [Native] Session property manager pass configs to bootstrap (prestodb…

841053f

…#25553)

Summary: Currently, session property manager has an unused parameter
called `properties` so that even if a config is set in a file, for
example `/etc/session-property-providers/native-worker.properties`, the
configs are not correctly used.

Differential Revision: D78415640

```
== RELEASE NOTES ==

General Changes
* Fix native session property manager reading plugin configs from file

lga-zurich pushed a commit to lga-zurich/presto-exchange that referenced this pull request


          [Native] Session property manager pass configs to bootstrap (prestodb…

6123cfd

…#25553)

Summary: Currently, session property manager has an unused parameter
called `properties` so that even if a config is set in a file, for
example `/etc/session-property-providers/native-worker.properties`, the
configs are not correctly used.

Differential Revision: D78415640

```
== RELEASE NOTES ==

General Changes
* Fix native session property manager reading plugin configs from file

lga-zurich pushed a commit to lga-zurich/presto-exchange that referenced this pull request


          [Native] Session property manager pass configs to bootstrap (prestodb…

c601ef0

…#25553)

Summary: Currently, session property manager has an unused parameter
called `properties` so that even if a config is set in a file, for
example `/etc/session-property-providers/native-worker.properties`, the
configs are not correctly used.

Differential Revision: D78415640

```
== RELEASE NOTES ==

General Changes
* Fix native session property manager reading plugin configs from file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

rschlussel rschlussel approved these changes

amitkdutta amitkdutta approved these changes

pdabre12 pdabre12 approved these changes

tdcmeehan tdcmeehan left review comments

jaystarshot Awaiting requested review from jaystarshot jaystarshot is a code owner

feilong-liu Awaiting requested review from feilong-liu feilong-liu is a code owner

elharo Awaiting requested review from elharo elharo is a code owner

vivek-bharathan Awaiting requested review from vivek-bharathan

Labels

fb-exported from:Meta