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

Raw Query construction for Request class #22

Closed
kentfredric opened this issue Oct 15, 2014 · 4 comments
Closed

Raw Query construction for Request class #22

kentfredric opened this issue Oct 15, 2014 · 4 comments

Comments

@kentfredric
Copy link
Contributor

This really aught to be a discussion of some kind, because I don't really know what I'm doing here.

request->ssearch() is presently poorly designed for doing raw queries.

You can hack around it with

request->ssearch('author', { bogus => 1 }, { %raw_query_here });

And it works because you can stuff {body => } there and blow away the annyoingly constrictive generated one.

But its ugly, and the function is not very obvious. You need the bogus stuff because without it, it simply breaks due to the generated query having $args != 1

I've considered various ways of hacking around it, to make the interface slightly more usable, for instance:

https://gist.github.com/kentfredric/22266f846eb6003dc842

But that solution strikes me as pretty dodgy because of the magical behaviour that is dependent on the value of $args

I even considered

diff --git a/lib/MetaCPAN/Client/Request.pm b/lib/MetaCPAN/Client/Request.pm
index 3c58fcf..2c20bb5 100644
--- a/lib/MetaCPAN/Client/Request.pm
+++ b/lib/MetaCPAN/Client/Request.pm
@@ -158,6 +158,9 @@ sub _build_query_rec {
     ref $args eq 'HASH' or croak 'query args must be a hash';

     my %query = ();
+
+    return \%query if not keys %{$args};
+
     my $basic_element = 1;

   KEY: for my $k ( qw/ all either not / ) {

But really, that's the same problem all over again, magical behaviour with a magic token.

I think this, slightly more complex approach would be better:

https://gist.github.com/kentfredric/65295334188a7fba4024

Simply because it keeps the existing API exactly the same, doesn't introduce any magical token behaviour, and gives a useful raw search at the same time.

Incidentally, that approach was the first one I tried, and the other ones are later ideas I had after I realised you could already pass custom params.

@haarg
Copy link
Member

haarg commented Oct 15, 2014

If you want a real API for this, can you propose a public API? All your last gist does is move code into a private sub, which doesn't seem reasonable for something you want as a public API.

@kentfredric
Copy link
Contributor Author

I think, maybe

   $REQUEST->raw_scroll_search(\%CONFIG)

Where %CONFIG is just content to pass to the ScrollSearch->new() method is the basic utility we're looking for here.

ie: Essentially, all I'm/we're looking for is a way to get an Elasticsearch scrolled query that we can manually bolt on to a ->ResultSet later the same way MetaCPAN::Client does.

The reason I chose a hash reference instead of a list is

  • Lower chance of that useless "unbalanced hash" warning occurring when internally casting as hash. ( Just simpler interface logic for us ).
  • And not entirely sure at this point that the direct hash list will be all we need.

Of course, this feature could possibly be complimented with a similar function at the MetaCPAN::Client level, but I'm not entirely certain that those are "consumer level" interfaces, but might be more "MetaCPAN::Client and freinds implementation level" interfaces.

And in that vein, in conjunction with bug #20, we could later add perhaps:

$mcpanclient->raw_scrolled_search({
       %args
});

Which returns a ResultSet that dispatches instances 'MetaCPAN::Client::' . uc %args{type}

And

$mcpanclient->raw_scrolled_search({
       class => "My::Class::Name"
       %args
});

Which returns a ResultSet that dispatches instances of My::Class::Name

But as I said, I'm aware some of these ideas are inconsistent with each other, and I'm not completely sure of half of my opinions yet.

Just the short of it is:

  • It is presently hard to perform arbitrary low level queries
  • Especially to get them to turn into arbitrary objects ( which may be simple subclasses of existing MetaCPAN::Client classes )
  • And these points limit development of extensions to MetaCPAN::Client

@oalders
Copy link
Member

oalders commented Oct 16, 2014

Being able to bypass the MetaCPAN::Client DSL would be a handy function to have. I like the DWIM aspect for sure, but if we can make it possible to issue arbitrarily complex queries as well which still return the resultset objects, that would solve a lot of problems, in much the same way that DBIx::Class allows raw SQL in queries. I think a discussion of raw_scrolled_search() would be a good start.

@mickeyn
Copy link
Contributor

mickeyn commented Apr 10, 2015

this is supported in modern versions

@mickeyn mickeyn closed this as completed Apr 10, 2015
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

4 participants