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

Use URI to connect to server #43

Closed
ghost opened this issue Aug 22, 2019 · 14 comments
Closed

Use URI to connect to server #43

ghost opened this issue Aug 22, 2019 · 14 comments
Assignees
Milestone

Comments

@ghost
Copy link

ghost commented Aug 22, 2019

It would be nice if the connect() has a variant that can distangle the URI for monetdb, e.g.
pymonetdb.connect(URI=mapi:monetdb://Martins-MacBook-Pro-3.local:50000/sf1)
This can be broken down into host,port, database

@gijzelaerr gijzelaerr self-assigned this Oct 29, 2019
@gijzelaerr gijzelaerr added this to the 1.3.0 milestone Oct 29, 2019
@gijzelaerr
Copy link
Collaborator

I propose {protocol}://{hostname}[:{port}]/{database} where [] indicates optional. The library probably should only work with the mapi:monetdb:// prefix, shall we make that optional also?

@kutsurak
Copy link
Member

I agree with the general proposal. I also am OK with having port and protocol specification optional.

URIs also allow for username/password specification: {protocol}://[username:password@]{hostname}[:port]/{database}, although especially for the password entry I am not sure this is a very good idea: Apparently the username:password format in the userinfo subcomponent has been deprecated: https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Generic_syntax

@gijzelaerr
Copy link
Collaborator

is the : in mapi:monetdb valid as schema?

i'm trying out the uritools library which is supposed to be RFC 3986 compatible, but it has issues with this syntax:

In [1]: from uritools import uricompose, urijoin, urisplit, uriunsplit

In [2]: q = 'mapi:monetdb://user:password@host.domain:50000/sf1'

In [3]: urisplit(q)
Out[3]: SplitResultUnicode(scheme='mapi', authority=None, path='monetdb://user:password@host.domain:50000/sf1', query=None, fragment=None)

In [4]: q = 'monetdb://user:password@host.domain:50000/sf1'

In [5]: urisplit(q)
Out[5]: SplitResultUnicode(scheme='monetdb', authority='user:password@host.domain:50000', path='/sf1', query=None, fragment=None)

@kutsurak
Copy link
Member

I am quoting from RFC 3986:

3.1. Scheme

Each URI begins with a scheme name that refers to a specification for
assigning identifiers within that scheme. As such, the URI syntax is
a federated and extensible naming system wherein each scheme's
specification may further restrict the syntax and semantics of
identifiers using that scheme.

Scheme names consist of a sequence of characters beginning with a
letter and followed by any combination of letters, digits, plus
("+"), period ("."), or hyphen ("-"). Although schemes are case-
insensitive, the canonical form is lowercase and documents that
specify schemes must do so with lowercase letters. An implementation
should accept uppercase letters as equivalent to lowercase in scheme
names (e.g., allow "HTTP" as well as "http") for the sake of
robustness but should only produce lowercase scheme names for
consistency.

So indeed the : character is not allowed in the protocol part. We could substitute it with a '.' maybe, although I am not sure what other protocol besides mapi would be meaningful here. Maybe versioned like mapi9 or mapi10, but I don't see the value in having monetdb there as well.

@gijzelaerr
Copy link
Collaborator

looks to me like the core monetdb developers need to make a decision here.

@gijzelaerr
Copy link
Collaborator

wouldn't it be better to have monetdb as the protocol name, since a regular user will not be aware of the fact he or she is using MAPI? Also, isn't the protocol initialisation step smart enough to distinguish between version 9 and 10?

@gijzelaerr
Copy link
Collaborator

@mlkersten @kutsurak I don't have any strong opinions about this, apart from that the monetdb:// seems to be the most intuitive syntax. What do you guys prefer?

@kutsurak
Copy link
Member

Honestly I would prefer the monetdb:// (or mapi9://, or mapi10://, or mapi://) scheme. I remember though last time we discussed this that @njnes had some arguments against it related to ODBC.

@njnes
Copy link
Contributor

njnes commented Jan 29, 2020

To stay within the odbc/jdbc uri scheme we should keep the mapi:monetdb:// etc here
jdbc:monetdb://[:]/[?=[&=]]

@gijzelaerr
Copy link
Collaborator

so the odbc/jdbc URI scheme violates RFC 3986?

@njnes
Copy link
Contributor

njnes commented Feb 3, 2020

I'm not sure thats how we should interpret the scheme: in this RFC. But as just discussed with Panos, we prefer (for pymonetdb) the sorter simple uri monetdb:// for example. So lets continue that way. That jdbc/odbc requires this is, shouldn't be a problem for the pymonetdb code. Seems we all agree now.

@sjoerdmullender
Copy link
Member

Keep in mind that the "mapi" scheme has two different types:
mapi:monetdb://localhost:50001/test?lang=sql&user=monetdb
mapi:merovingian://proxy?database=test

With this in mind, the URI parses as
scheme: mapi
hier-part: monetdb://localhost:50001/test or merovingian://proxy
query: lang=sql&user=monetdb or database=test

The hier-part cannot be further broken down according to the generic syntax, so the authority that is defined there does not apply.

Perhaps it would be nice if it did, but then we cannot use the parts "monetdb:" and "merovingian:".

If we are to simplify the scheme so that we can use the generic syntax for the authority (i.e. username, hostname, port) we need to first remind ourselves how the monetdb and merovingian subschemes are used, and then how we can change those.

@gijzelaerr
Copy link
Collaborator

ok, then I'm still not really sure what the consensus here is. Since I want to push out 1.3.0 out today, I'm moving this discussion to 2.0.0 since this is the last blocking issue.

@gijzelaerr gijzelaerr modified the milestones: 1.3.0, 2.0.0 Feb 18, 2020
@joerivanruth
Copy link
Member

I was unaware of the discussion in this ticket but last year I submitted a pull request adding support for mapi: urls and Gijs merged it, see #89.

The PR implemented a libmapi-like mapi:monetdb://... syntax so you can copy/paste from monetdb status.

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

No branches or pull requests

5 participants