The ruby driver silently ignores unknown optional arguments

[AtnNn]

This confused a user that was using camel case.

> r.connect(auth_key:"hunter2")
#<RethinkDB::Connection:0x00000001d67760>
> r.connect(authKey:"hunter2")
ERROR: incorrect authorization key

@mlucy pointed out that it would be best to fix this on server, which also silently ignores unknown optional arguments.

[mlucy]

I think this check should be done on the server rather than in the clients. That way third-party drivers get it for free.

[coffeemug]

Assigning to @gchpaco

[mlucy]

Note that optargs passed to connect need to be handled in the driver, but unknown global optional arguments should probably be rejected on the server.

[gchpaco]

Fix for the global optargs thing in CR 2436.

[gchpaco]

Should be solved now.

[mlucy]

Sorry to resurrect a dead horse, but re-opening briefly to talk about the set of accepted global optargs.

The linked patch makes some global optargs which are currently accepted illegal. I'm not necessarily opposed to doing this, but it seems like a bad idea on the surface: it might break somebody's app and there's no real cost to accepting them. I could very easily see someone wanting to specify e.g. geo_system globally. It also seems strange that for some terms we now accept some but not all of their optargs at the global level. For example, you can set durability at the top level, but not non_atomic. I can imagine that being confusing.

[danielmewes]

The current list is based on what the JavaScript driver used to allow as global opt args.

I personally am neutral on the question of whether to allow all opt args globally or only the selected ones.

[gchpaco]

The JS driver actually used to reject some legal global optargs, because I forgot to update it for the batch changes. But that's where the list came from.

[danielmewes]

Pro accepting all opt args:

• backwards compatibility for Ruby and Python
• no arbitrary distinction (e.g. durability)
• more flexbility when writing queries (can also save some typing)

Contra accepting all opt args:

• really easy to forget adding a new opt arg to the list in the future
• having to specify opt args at the term where they are being used makes sure people understand which terms a given opt arg has effects on

[gchpaco]

Further contra: no anti-typo protection.

[danielmewes]

@gchpaco By "accepting all opt args" I meant accepting all opt args that are accepted by at least one of the terms. So we would just extend the list, definitely not revert your change.

[gchpaco]

OK.

[mlucy]

really easy to forget adding a new opt arg to the list in the future

If we do this, we should make the optargspec_t constructor guarantee that the optarg is in the global list. That way we'd get a crash the first time we tried to test a term using the new optarg.

having to specify opt args at the term where they are being used makes sure people understand which terms a given opt arg has effects on

Yeah. There are also some optargs which are just outright confusing at the toplevel (timeout doesn't specify a timeout for the whole query, it specifies the timeout for any r.js calls in the query).

[danielmewes]

If we do this, we should make the optargspec_t constructor guarantee that the optarg is in the global list. That way we'd get a crash the first time we tried to test a term using the new optarg.

Ah that's a good idea.
With that out of the way, I'm leaning towards having all opt args that are allowed somewhere to also be allowed globally.

[gchpaco]

Okay.

[mlucy]

With that out of the way, I'm leaning towards having all opt args that are allowed somewhere to also be allowed globally.

That sounds slightly better to me too. All hail backwards compatibility!

[gchpaco]

CR 2456.