Skip to content

Decrease size of product-tests-launcher jar#14974

Merged
losipiuk merged 1 commit intotrinodb:masterfrom
losipiuk:lo/smaller-launcher
Nov 10, 2022
Merged

Decrease size of product-tests-launcher jar#14974
losipiuk merged 1 commit intotrinodb:masterfrom
losipiuk:lo/smaller-launcher

Conversation

@losipiuk
Copy link
Copy Markdown
Member

@losipiuk losipiuk commented Nov 9, 2022

Decrease size of product-tests-launcher jar## Release notes

(s) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

@cla-bot cla-bot bot added the cla-signed label Nov 9, 2022
@losipiuk losipiuk requested a review from findepi November 9, 2022 16:13
Comment on lines 26 to 29
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, by their design, prroduct tests should be "black box". They intentionally never depended on the trino-main.
Can you perhaps enhance the comment to indicate that too?

Also, why does the plugin reader pull trino-main?
Is it a fake dep? Does plugin reader combine different functionalities, some depending on trino-main and some not? maybe plugin reader should be two separate modules?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PluginReader uses this single function from trino-main - extractFunctions: https://github.com/trinodb/trino/blob/master/testing/trino-plugin-reader/src/main/java/io/trino/server/PluginReader.java#L136

The product tests launcher only needs a few constants from the plugin reader. It definitely feels like something should be moved to improve dependencies, but I'm not sure what.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The product tests launcher only needs a few constants from the plugin reader. I

Can we move these constants out to trino-plugin-reader-{api,spi,something} jar?

Copy link
Copy Markdown
Member Author

@losipiuk losipiuk Nov 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - it crossed my mind too. I was not convinced that we want extra module just for that. Probably the cleanest approach still.
I will merge this one as a stop-gap though.

@losipiuk losipiuk force-pushed the lo/smaller-launcher branch from f4df17d to fcbb44b Compare November 10, 2022 07:42
@losipiuk losipiuk merged commit f60bc22 into trinodb:master Nov 10, 2022
@github-actions github-actions bot added this to the 403 milestone Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants