xsawyerx / metacpan-api

A comprehensive, DWIM-featured API to MetaCPAN
http://search.metacpan.org
14 stars 14 forks source link

Make base_url 'rw' #1

Closed oalders closed 13 years ago

oalders commented 13 years ago

Hi Sawyer,

After looking at your code, I realized that the dist search URLs are not documented in the API. However, if you're planning to move this over to SPORE, then it probably makes sense to add those URLs at that point. The current "search_dist" method might need to be something like " search_modules_by_dist" or "search_modules_in_dist" once the actual search_dist URL gets added. :)

xsawyerx commented 13 years ago

Merged, thanks! :)

Hopefully get to update new version this weekend (maybe even today).

xsawyerx commented 13 years ago

I've decided not to move this to SPORE, since SPORE uses Moose and I want something very small, to be very flexible. Making it big means it will not be used easily in command line applications (which is my first requirement for it).

BTW, are you positive the base_url should be 'rw'? I'm not so certain about it.

oalders commented 13 years ago

Great! I'll watch for an updated version then.

The idea with MetaCPAN is that it should be possible for someone to host their own instance of it (for whatever reason), so I think the base_url can be flexible. Also, when I wanted to use MetaCPAN::API to test the coverage of my developer instance, I wasn't able to do it without changing the base URL, since my instance runs on http://localhost:9200.

I think we'd make nightly snapshots available so you could host your own MetaCPAN without needing to be online etc. Anyway, this is a perfect tool for testing the ES index before it gets ported to the live machine, but some flexibility on the base_url would be required for that.

Also, one other issue is that the URLs in the wiki are "pretty" URLs which we've cleaned up a touch with some rewrites in nginx. We might want to consider having the URLs under the hood use the full ES urls. Something like http://api.metacpan.org:9200/cpan/dist/Dancer rather than http://api.metacpan.org/dist/Dancer The reason for that is that the URLs on port 9200 allows for some more complex syntax which you can't do when using the rewritten URLs. So, if users have access to the HTTP::Tiny object, they can extend things a bit without the URL scheme getting in the way.

I can help with that once you've updated the code. It's pretty simple, but I think it would be helpful. Of course, this isn't in the Wiki docs yet. ;)

xsawyerx commented 13 years ago

I've refactored quite a bit in this development release. If it's goes well, I'll release it and start writing tests for it.

The idea with the base_url immutability is that you create a new MetaCPAN::API instance for any base_url you want to use. We should sort it out before people start writing code that is assuming on the 'rw' status of base_url.

I'll also add the ability to give specific args for HTTP::Tiny, in an immutable manner. That is, only at the init of MetaCPAN::API.

oalders commented 13 years ago

Ah, I understand. So you're saying you set an arbitrary base_url on init and it's immutable from that point on. That makes perfect sense. In that case, please uncommit my commit. :)

xsawyerx commented 13 years ago

Thanks for understanding.

I assure you that if ever this would be a real problem, I won't resist changing it. :)

oalders commented 13 years ago

I think the real problem is my understanding of the difference between 'ro' and 'rw'!

xsawyerx commented 13 years ago

It was quite confusing to me at first!

Mainly, 'ro' means that you get no setter for this attribute. So, you cannot do $object->attr($new_value). This does not affect in any way the ability to set a value for it on initialize: my $object = Class->new( attr => $value );

oalders commented 13 years ago

When you put it that way, it makes perfect sense. Thanks for making that click for me. :)