Skip to content

Conversation

@christyjacob4
Copy link
Contributor

@christyjacob4 christyjacob4 commented Oct 25, 2022

Adds support for a new DSN class that allows us to easily parse and manage Data Source Names.
Open questions

  • In-order for this library to be tailored specifically for Database and Storage, we might need additional functions like getDatabase(), getRegion(), getBucket() etc. What would be an ideal place to add these ? Within this library or in the consuming client ?

*/
public function getQuery(): ?string
{
return $this->query;
Copy link
Member

Choose a reason for hiding this comment

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

For some more complex use cases we might want to add something like getParam to get a specific value from the query string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a great idea!

I've implemented the getParam function. Should it also accept a default value that can be returned if the key does not exist? If yes should we assume this type of the default value to be a string ?

Copy link
Member

Choose a reason for hiding this comment

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

I think a string default makes a lot of sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good 👍

@eldadfux
Copy link
Member

In-order for this library to be tailored specifically for Database and Storage, we might need additional functions like getDatabase(), getRegion(), getBucket() etc. What would be an ideal place to add these ? Within this library or in the consuming client ?

This library should definitely stay focused on the DSN protocol and not have any specific references. Actually, I think the DNS format is powerful enough and don't really see the need in specific methods for specific use cases.

I think our specific Utopia lIbraries should avoid using this library as a direct dependency. Our servers that need to have different connections can use this library to connect env vars to different utopia adapters that are using some kind of a connection.

@eldadfux eldadfux merged commit 17a5935 into main Oct 26, 2022
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.

3 participants