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

feat: language plugins require a bind allocator to be provided #3069

Merged
merged 7 commits into from
Oct 10, 2024

Conversation

matt2e
Copy link
Contributor

@matt2e matt2e commented Oct 10, 2024

This is a prerequisite for external language plugins. This PR makes sure languageplugin.New() is called with a bind allocator so that external plugins will soon able to able to acquire a bind url.

@matt2e matt2e requested review from a team and alecthomas as code owners October 10, 2024 04:43
@matt2e matt2e requested review from wesbillman and removed request for a team October 10, 2024 04:43
@matt2e matt2e force-pushed the matt2e/plugins-need-bind-allocator branch from 24278b1 to 717b2a9 Compare October 10, 2024 04:43
@ftl-robot ftl-robot mentioned this pull request Oct 10, 2024
@@ -1,7 +1,7 @@
env = {
"DBMATE_MIGRATIONS_DIR": "${HERMIT_ENV}/backend/controller/sql/schema",
"DBMATE_NO_DUMP_SCHEMA": "true",
"FTL_ENDPOINT": "http://localhost:8892",
"FTL_ENDPOINT": "http://127.0.0.1:8892",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This prevents MacOS from showing alert dialogs for incoming connections

@matt2e matt2e marked this pull request as draft October 10, 2024 04:46
@matt2e matt2e marked this pull request as ready for review October 10, 2024 05:31
@matt2e matt2e force-pushed the matt2e/plugins-need-bind-allocator branch from d50685e to 6e63007 Compare October 10, 2024 06:08
Copy link
Collaborator

@alecthomas alecthomas left a comment

Choose a reason for hiding this comment

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

I really would love to not have to use the port allocator, but I couldn't think of a better way. Connect doesn't support Unix sockets, which would have been ideal 🙁

@matt2e matt2e force-pushed the matt2e/plugins-need-bind-allocator branch from 6e63007 to 1f7c673 Compare October 10, 2024 21:58
@matt2e matt2e enabled auto-merge (squash) October 10, 2024 21:58
@matt2e matt2e merged commit 9300e48 into main Oct 10, 2024
95 checks passed
@matt2e matt2e deleted the matt2e/plugins-need-bind-allocator branch October 10, 2024 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants