Conversation
ed174de to
a17ec82
Compare
52c4a14 to
884ae23
Compare
a17ec82 to
20c9a3a
Compare
280e89e to
1e776a2
Compare
1e776a2 to
b2e77c6
Compare
MarioCadenas
left a comment
There was a problem hiding this comment.
In general code LGTM, I'm not so sure about the onSetupMessage thing though, let's discuss that
| "description": "SPDX license identifier", | ||
| "examples": ["Apache-2.0", "MIT"] | ||
| }, | ||
| "onSetupMessage": { |
There was a problem hiding this comment.
so if we have multiple plugins, with the onSetupMessage, then we will have multiple instructions printing? also, this seems like something needed just for the lakebase plugin and not for a general use case, what other use cases do you see?
There was a problem hiding this comment.
so if we have multiple plugins, with the onSetupMessage, then we will have multiple instructions printing?
For now it is used for Lakebase, but I can imagine other plugins that require some manual steps - something what CLI / AppKit cannot do automatically - for example, an external service setup "Create a Slack webhook and set env SLACK_WEBHOOK_URL"
I can see value especially in the external / third party plugins. I totally agree that with our plugins we should aim to not print such messages and have everything provided out of the box.
This is completely optional for plugins, and it doesn't change the goal of having the plugins zero-config 👍
0f3f5c5 to
604ed6c
Compare

This PR introduces a thin wrapper on top of the Lakebase driver, to ensure consistent behavior.
Resolves https://databricks.atlassian.net/browse/LKB-9544
Notes
Should we really name it as "Lakebase"? Victor suggested using name "Transaction" but it doesn't resonate with me.
On the other hand,
oltp,database,postgresdon't sound like better options thanlakebase🤔 We need to distinguish this plugin with theanalyticsplugin somehow.IMO the "Lakebase" name is the best what we can do right now, even though it is not consistent with
analyticsplugin. Let's see how the other plugins are named and then rename it - or, if you have any ideas, feel free to suggest them already.All PRs
apps initflow, remove empty files after template rendering cli#4549