Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

Issue 612: user /v1/eval for ML8+ #613

Merged
merged 3 commits into from
Jul 11, 2016
Merged

Conversation

dmcassel
Copy link
Collaborator

Addresses #612 and #596. Uses /v1/eval for MarkLogic 8 and MarkLogic 9.


r = go "#{@protocol}://#{@hostname}:#{@qconsole_port}/v1/eval", "post", headers, params

raise ExitException.new(JSON.pretty_generate(JSON.parse(r.body))) if r.body.match(/\{"error"/)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you verify that this line works as intended? I'm ignorant of the output of /v1/eval.

@grtjn
Copy link
Contributor

grtjn commented May 25, 2016

I'm amazed, is that all? I'm kinda missing context like db or app-server selection. We have a lot of code that picks targets explicitly. Does that still work?

I guess we could run self-test to see what comes out of that, but also thinking of app_specific, in which exec of queries is used a lot..

@grtjn
Copy link
Contributor

grtjn commented Jun 14, 2016

/v1/eval returns multipart/mixed response. We need to parse that similarly to how we were parsing the json response from the newer QC. I ran a test and this code seemed to work nicely, which I added to util.rq:

def parse_multipart(body)
  if (body.match("^\r\n--"))
    # Extract the delimiter from the response.
    delimiter = body.split("\r\n")[1].strip
    parts = body.split(delimiter)

    # The first part will always be an empty string. Just remove it.
    parts.shift
    # The last part will be the "--". Just remove it.
    parts.pop

    # Get rid of part headers
    parts = parts.map{ |part| part.split("\r\n\r\n")[1].strip }

    # Return all parts as one long string, like we were used to.
    return parts.join("\n")
  else
    return body
  end
end

def parse_body(body)
  parse_multipart(parse_json(body))
end

And then replace references to parse_json with parse_body in server_config.rb (dozen times or more), and test_server_config.rb (only once). Note: not in editor.rb, that has its own parse_json. I think that has a different purpose..

This should be enough to at least get self-test working. Worked for me locally.

@grtjn
Copy link
Contributor

grtjn commented Jun 14, 2016

Still want to look a bit closer to that context stuff. Stay tuned..

@grtjn
Copy link
Contributor

grtjn commented Jun 14, 2016

I searched through the Roxy code, and the app_name parameter is not used internally. db_name is though, so we need to leverage the database param of /v1/eval..

@grtjn
Copy link
Contributor

grtjn commented Jun 14, 2016

I filed an RFE to request server context support for /v1/eval. In meantime, I suggest we add the following to the top of execute_query_8:

    if properties[:app_name] != nil
      raise ExitException.new("Executing queries with an app_name (currently) not supported with ML8+")
    end

And for database support we only need this between params = .. and r = go ..:

    if properties[:db_name] != nil
      params[:database] = properties[:db_name]
    end

@grtjn grtjn changed the title Issue 612 Issue 612: user /v1/eval for ML8+ Jun 20, 2016
@grtjn
Copy link
Contributor

grtjn commented Jun 20, 2016

@dmcassel Did you incorporate my suggestions yet?

@dmcassel
Copy link
Collaborator Author

dmcassel commented Jul 8, 2016

Okay, I made @grtjn's suggested updates. self-test on ml7 ran successfully, passed my testing on ml8 and ml9.

@dmcassel dmcassel assigned dmcassel and grtjn and unassigned dmcassel Jul 8, 2016
@grtjn
Copy link
Contributor

grtjn commented Jul 11, 2016

Commits look clean, I'll try to give it a spin asap..

@grtjn
Copy link
Contributor

grtjn commented Jul 11, 2016

Yep, self-test runs clean at my end too..

Just wondering, we may wanna apply parse_body to the result of eval itself as well, otherwise this might fail?

raise ExitException.new(JSON.pretty_generate(JSON.parse(r.body))) if r.body.match(/\{"error"/)

(this line exists in execute_query_5, execute_query_7, and execute_query_8)

@dmcassel
Copy link
Collaborator Author

@grtjn I set up bootstrap to fail a couple different ways, but saw no change when I modified or even removed that line from execute_query_8. The errors were displayed correctly. Not sure what kind of error gets to that line, but it doesn't seem to need the change.

@grtjn
Copy link
Contributor

grtjn commented Jul 11, 2016

good enough for me..

@grtjn
Copy link
Contributor

grtjn commented Jul 11, 2016

let's wait no longer, merging now..

@grtjn grtjn merged commit 72d0fd9 into marklogic-community:dev Jul 11, 2016
@dmcassel
Copy link
Collaborator Author

Thanks @grtjn!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants