Color required fields in red on database connection

[josevalim]

@cristineguadelupe is adding color-coded required fields to kino_map_libre and we should probably follow suite here:

For MySQL/Postgres, both host+port are required so they should be colored in red, even if prefilled.

For SQLite3, the file path is required.

For GoogleBigQuery, both project id and dataset are required.

For Athena, I think all five fields are required and none of them have a reasonable default.

We should also check for each of those what happens if not all fields are filled and we try to execute it. Is the error message at least decent? Otherwise we need to make sure the underlying libraries raise a reasonably good error message. :)

[aleDsz]

@wojtekmach WDYT?

1. Add Req.Request.register_required_options/2 to register required options to fail on attach/2
2. Control the required options directly from plugin (attach/2)

[cristineguadelupe]

I’m completely out of touch with Kino DB, so just ignore if what I’m saying doesn’t make sense here, but what I’m doing in Kino Maplibre is validating into_source and not generating the code as long as there’re any required field unfilled

[cristineguadelupe]

And I’m not sure I understand what @josevalim meant by prefilled, but if there’s a valid value in the field, even if it’s the default, it shouldn't be red, imo

[aleDsz]

And I’m not sure I understand what @josevalim meant by prefilled, but if there’s a valid value in the field, even if it’s the default, it shouldn't be red, imo

The port field is prefilled with default port for some databases (mysql and postgres), so even they have their defaults prefilled, we should require the port field and validate it

[aleDsz]

@josevalim For Athena, we could use the region us-east-1 as default, WDYT?

[wojtekmach]

Add Req.Request.register_required_options/2 to register required options to fail on attach/2
Control the required options directly from plugin (attach/2)

whether something is required or not starts to get into territory of tools like NimbleOptions so I wouldn't want to pursue it at the moment but will definitely keep it in mind as we go. For now, plugin authors need to do the validations manually themselves. I think most of the time folks wouldn't do validations in attach/2 as plugin can be attached but ultimately not used. So I personally would do validations in a step. But nothing stops you from doing that, attach/2 is just a convention after all.

[aleDsz]

Add Req.Request.register_required_options/2 to register required options to fail on attach/2
Control the required options directly from plugin (attach/2)

whether something is required or not starts to get into territory of tools like NimbleOptions so I wouldn't want to pursue it at the moment but will definitely keep it in mind as we go. For now, plugin authors need to do the validations manually themselves. I think most of the time folks wouldn't do validations in attach/2 as plugin can be attached but ultimately not used. So I personally would do validations in a step. But nothing stops you from doing that, attach/2 is just a convention after all.

If we try to not send the options from attach/2 to ReqAthena/ReqBigQuery, it'll raise since we are trying to access a key from a map that doesn't exist, but the error message couldn't be more clear for some users.

So, the best option for Livebook is to not generate the code from the connection cell (as @cristineguadelupe is doing with Kino Maplibre) until all required fields are filled. But, for that, we should stop sending empty inputs as empty string to KinoDB.

[jonatanklosko]

But, for that, we should stop sending empty inputs as empty string to KinoDB.

What do you mean? We can just check if the required fields are not empty when generating the code.

[aleDsz]

But, for that, we should stop sending empty inputs as empty string to KinoDB.

What do you mean? We can just check if the required fields are not empty when generating the code.

Yeah, this works too. What I meant is to not send empty values from main.js