ruby-rdf / sparql-client

SPARQL client for Ruby.
http://rubygems.org/gems/sparql-client
The Unlicense
112 stars 58 forks source link

Update requests parse the result for no purpose #71

Closed cecton closed 8 years ago

cecton commented 8 years ago

Hello,

I'm already sorry if this question is totally irrelevant. I'm don't know Ruby at all but I show the issue to a few Ruby developers and none of them found a solution to it.

Here is the piece of code that is puzzling me in the SPARQL::Client:

    def update(query, options = {})
      @op = :update
      @alt_endpoint = options[:endpoint] unless options[:endpoint].nil?
      case @url
      when RDF::Queryable
        require 'sparql' unless defined?(::SPARQL::Grammar)
        SPARQL.execute(query, @url, options.merge(update: true))
      else
        parse_response(response(query, options), options) # <------- here
      end
      self
    end

As you can see. The result of the request is parsed but it is not stored and I believe it is not accessible to the caller since this method returns self.

Is there some magic somewhere that allows retrieving the parsed result?

Thanks

gkellogg commented 8 years ago

Good question; perhaps the parse_response can be left out, if it is not returned. Perhaps this should raise an exception on an unexpected response. The return self is a common pattern for such methods.

cecton commented 8 years ago

Personally I would remove the parsing. It looks like dead code and a user wouldn't expect the response to be parsed if they can't access it anyway. But it is up to you to decide. I will be happy to create the PR of course 😄

My experience with this issue is that I'm working with Sesame and it fails because the updates on Sesame return a 204 No Content and the fix that handles that issue is in the branch develop. So it's blocking without a good reason to block.

no-reply commented 8 years ago

:+1: to removing the #parse_response call.