IQSS / dataverse

Open source research data repository software
http://dataverse.org
Other
882 stars 493 forks source link

Have PUT admin/settings/$name/$content accept data via request body #1329

Closed michbarsinai closed 9 years ago

michbarsinai commented 9 years ago

More HTTP-like, rather than putting it in the url.

michbarsinai commented 9 years ago

setup-all.sh and setup-users.sh updated to support the new method.

kcondon commented 9 years ago

@michbarsinai Can you give me an idea of what this means? I can just close the ticket since setup-all is still working but would be nice to know what changed. Thanks!

michbarsinai commented 9 years ago

A http PUT request to a given URL u, means "put the request body under u". We didn't do this, but rather had a hack that was simpler to write at the time. We used:

curl -X PUT http://server/api/s/settings/setting_name/setting_content

Note that that URL contains both the name and the content, and that the request body is empty. Side effects of this is that the content has to be URL-encoded, and that it was limited in length.

We now use:

curl -X PUT -d content http://server/api/s/settings/setting_name`

So the content is passed as-is in the body of the request.

pdurbin commented 9 years ago

curl -X PUT -d content http://server/api/s/settings/setting_name`

We should make sure http://guides.dataverse.org/en/latest/installation/installation-main.html has been updated.

kcondon commented 9 years ago

Ok, thanks Michael, closing

kcondon commented 9 years ago

Creating a separate ticket for documenting the settings api in install guide and updating the examples to reflect this new format.

pdurbin commented 9 years ago

curl -X PUT -d content http://server/api/s/settings/setting_name

@michbarsinai I like where this ticket is going in general but I'm confused why the data to pass in is not in JSON format. It's just a string? Seems weird. Can we use JSON instead?

michbarsinai commented 9 years ago

Hmmmmmm.... I think not. Here's the reason: This is a basically a persistent hash map. Give it a key and a value, and it will store it and be able to retrieve it. It does not look into the values it stores, and cannot manipulate them. It is used, so far, for strings that are not even parsed, but used as-is:

dvndb=# SELECT * FROM setting ;
       name       |               content
------------------+--------------------------------------
 :AllowSignUp     | yes
 :SignUpUrl       | /dataverseuser.xhtml?editMode=CREATE
 :Protocol        | doi
 :Authority       | 10.5072/FK2
 :DoiProvider     | EZID
 :DoiSeparator    | /
 BuiltinUsers.KEY | burrito

So I don't see the gains in using JSONs. The downsides are that it would make supporting other formats clumsy, e.g. if someone wants to use XML, or even CSV (where we'll need to escape the double quotes).

So my gut feeling is "no". Happy to be convinces otherwise, though.

scolapasta commented 9 years ago

I'm satisfied by this explanation. @pdurbin I leave it assigned to you, in case you have any objection, but otherwise, please pass back to QA.

pdurbin commented 9 years ago

@scolapasta I think we should have consistency in our native API (i.e. JSON input) but perhaps a ticket could be created for this for after 4.0. I'll give this to QA for any thoughts on this. Again, certainly the native API is better that we're PUTing data now.

kcondon commented 9 years ago

Closing