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

Fixing Issue #521: support custom properties for CoRB2 #626

Merged
merged 3 commits into from
Jul 19, 2016

Conversation

hansenmc
Copy link
Member

@hansenmc hansenmc commented Jul 14, 2016

Updated the corb task to use CoRB2 v2.3.1 and support all of the CoRB2 options.

Ensured that the current Roxy commandline options and functionality still work,
and also support specifying any of the CoRB2 options with either "--" or "-D"
prefix, case-insensitive (i.e. --options-file or -DOPTIONS-FILE).

Removed the explicit checks/errors for Roxy corb task required fields, since there
are now additional options available: URIS-FILE, XML-FILE, PROCESS-MODULE (now
preferred instead of XQUERY-MODULE), specifying all options inside of
OPTIONS-FILE, etc.

Also upgraded the XCC jar to v8.0.5
#521

…oRB2

Updated the corb task to use CoRB2 v2.3.1 and support all of the CoRB2 options.

Ensured that the current Roxy commandline options and functionality still work,
and also support specifying any of the CoRB2 options with either "--" or "-D"
prefix, case-insensitive (i.e. --options-file or -DOPTIONS-FILE).

Removed the explicit checks/errors for Roxy corb task required fields, since there
are now additional options available: URIS-FILE, XML-FILE, PROCESS-MODULE (now 
preferred instead of XQUERY-MODULE), specifying all options inside of 
OPTIONS-FILE, etc.

Also upgraded the XCC jar to v8.0.5
options["MODULES-DATABASE"] = modules_database
end

# A full list of CoRB2 options
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering, instead of adding knowledge of all Corb2 options inside Roxy, isn't is easier to just pass through whatever gets specified, and let Corb squeak itself if something is wrong? A bit like how we did things for MLCP, in which we more or less only append user/pwd/etc, and parse options files for properties replacement.

@grtjn
Copy link
Contributor

grtjn commented Jul 14, 2016

Thanks for the PR! Wondering though if we could do with less lines. See inline comment..

@hansenmc
Copy link
Member Author

I like that idea, both to reduce the lines and to make maintenance easier (won't need to update Roxy to enable new CoRB2 switches, simply drop in newer jar). I'll take a look and make the necessary updates.

Remove hard-coded list of option names, simply collect all args, remove
the “—“ or “-D” prefix, and ensure that the option name is UPPER-CASE
@grtjn
Copy link
Contributor

grtjn commented Jul 15, 2016

Looking great at first glance!

@grtjn grtjn added this to the 1.7.4 milestone Jul 15, 2016
xcc_file = find_jar('xcc')

runme = %Q{java -cp #{corb_file}#{path_separator}#{xcc_file} #{systemProperties} com.marklogic.developer.corb.Manager #{connection_string}}
logger.info runme
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change this into logger.debug? You then only see that when you use -v..

@grtjn
Copy link
Contributor

grtjn commented Jul 15, 2016

Just two small suggestions, and I think this is good to go..

@grtjn
Copy link
Contributor

grtjn commented Jul 15, 2016

PS: I created a ticket against Corb2 to improve usage printing to console. I like the way MLCP does it very much..

@grtjn
Copy link
Contributor

grtjn commented Jul 15, 2016

I'd be happy to merge this now. Ran a superficial test, and invoking Corb2 (to get a message on screen) succeeded. Low impact too. @paxtonhare / @dmcassel agree?

I'm only wondering if it is worth holding of for marklogic-community/corb2#31 to be fixed, so we can integrate a slightly newer Corb2 jar with better usage console output..

@dmcassel
Copy link
Collaborator

I think this looks good (haven't tested). Is this change backwards compatible?

@dmcassel
Copy link
Collaborator

Regarding marklogic-community/corb2#31, I don't see any comments there suggesting when it might get done. I agree it would be helpful, but I would suggest not waiting on it unless a short timeline gets proposed.

@hansenmc
Copy link
Member Author

Corb2 is mostly backwards compatible, except the built-in uris module is no longer provided. I tried to make it backwards compatible by continuing to support roxy corb params that did not already get mapped or have the same defaults, and automatically added an inline query when collection param was used.

I'll look at improving the corb2 exception. We could try toget it in and out out a maintenance release, if you need/want. Otherwise, we don't have a release planned in the immediate timeframe.

@dmcassel
Copy link
Collaborator

Sounds good to me. @grtjn you ran some tests; care to do the honors?

@grtjn
Copy link
Contributor

grtjn commented Jul 19, 2016

Merging now.

@hansenmc I leave it up to you to do a PR with a newer Corb2 when it comes out. Fixing the console output would help end users a lot though, so I'd suggest not waiting long with that..

@grtjn grtjn merged commit 54c1f74 into marklogic-community:dev Jul 19, 2016
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