Fix adapter finding

[ClaytonJY]

To reproduce, checkout the new-demos branch, build the package, then run

demo(test, package="RQt")

You'll have to hit enter to start it; it should run "successfully", but the rqt.runProcess call does not return what we want:

> rqt.runProcess(engine)
[1] "Could not read the input stream: /home/cyochum/config/adapters.xml (No such file or directory)"

That's definitely the wrong path to be looking in, but I have no idea how we could tell Java where to look. You can see in demo/test.R I use find.package(), but I don't know how to pass that to Java.

This wasn't caught by R CMD check because that outcome looks successful to R since no error was raised.

I don't know how to fix this, but this is pretty serious. I suspect the way we're using config is not ideal, but I don't know where else to put that stuff. Perhaps as a short-term workaround you can edit the appropriate java functions to take in a directory parameter, and then in R we could hardcode those to find.package("RQt") inside the R functions?

[javadch]

1. when it worked on other branches, how is it possible to not work here?
2. On my machine:
   1. the extdata should have a tailing forward slash -> extdata/
   2. it complained about using 'b' in the BIND statement in the test.R. I changed it to b2 and it works!
3. As a clue: in the rqt.getEngine() code, there is a call to this: getEngineReturnValue <- .jnew("xqt/api/LanguageServicePoint", "RQt, RQt/inst, inst")
   which means, the LanguageServicePoint function accepts the candidate locations for the config file. with these feature you have thee options,
   1. make a string containing "RQt, RQt/inst, inst" + find.package("RQt") and pass it to the aboved mentioned function.
   2. create a copy of rqt.getEngine(), names it like rqt.getEngine2(path) accepts the path you want, and pass it to the .jenw(.. instead of "RQt, RQt/inst, inst"
   3. pass find.package("RQt") result to the ,.jnew "RQt, RQt/inst, inst", without creating another function
      By the way, I do not think that the problem is with the path,...

[ClaytonJY]

1. RQt has never worked for me on any branch, so I don't know; we just got the Java issues worked out on Monday, after all.
2. On my machine,
   1. having the extra "" in file.path is to ensure dir ends with a slash; is that not the case on your machine (try running just that line to see the output)? Really, XQt should not be so picky, and assume that SOURCE_URI is a directory regardless of trailing slash.
   2. I get no complaint about b, unless it's somehow being masked. Why would that matter, is b a reserved keyword?
3. Those are definitely the wrong things to be passing to getEngine as inst/ does not exist, and RQt cannot be assumed to be in the current directory. I also think getEngine should be validating this input rather than waiting for a run-time XQt error.
   I say the problem is with the path because it's clearly looking in the wrong path; there is no expectation that /home/cyochum/config/ exists.
   (iii) seems like the only workable option to me, but I have no time to implement and test it.

[javadch]

The matter is that the source_uri is not always a folder. It can be a file
or a url too. I would go to make it smarter by the way.

Just proceed with what is workable and we decide what to do after my
presentation.
On Aug 27, 2015 10:41 AM, "Clayton" notifications@github.com wrote:

1. RQt has never worked for me on any branch, so I don't know; we just
   got the Java issues worked out on Monday, after all.
2. On my machine, having the extra "" in file.path is to ensure dir
   ends with a slash; is that not the case on your machine (try running just
   that line to see the output)? Really, XQt should not be so picky, and
   assume that SOURCE_URI is a directory regardless of trailing slash.
3. Those are definitely the wrong things to be passing to getEngine as
   inst/ does not exist, and RQt cannot be assumed to be in the current
   directory. I also think getEngine should be validating this input rather
   than waiting for a run-time XQt error. I say the problem is with the path
   because it's clearly looking in the wrong path; there is no expectation
   that /home/cyochum/config/ exists. (iii) seems like the only workable
   option to me, but I have no time to implement and test it.

—
Reply to this email directly or view it on GitHub
#30 (comment).

[ClaytonJY]

I have a full schedule this evening, so I can't help; you'll have to do what you can to get it ready for the morning.

[javadch]

The result of the find.package() can be passed to the .jnew call inside the getEngine function to replace the literal strings there

[ClaytonJY]

So in looking at this again, I also get a complaint about b; changing to b1 fixes that. Now I get a new error:

> rqt.runProcess(engine)
Note: /Stmt_1_Reader.java uses unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
[1] "**************************************************************************************\n****************************** Statement Execution Results ********************************\n**************************************************************************************\nStatement 1 was executed. Its result is in the variable: 'var3' and contains 5 records.\n"

Not only is there no var3 in my environment, such a side-effect is not what I would expect; I think rqt.runProcess should return the result of running the process.

Any idea what's going on here @javadch?

[ClaytonJY]

Oddly, I get that error whether or not I fix what's being passed to .jnew inside of rqt.getEngine, which was not the case last week. It's like that path doesn't matter anymore? Not sure how it's finding the adapter it couldn't find before.

[javadch]

It is indeed not an error. Its a compilation switch i have to set at the query translation time to suppress the message.
Results are kept in variables because processes are allowed to have multiple statements.

[javadch]

Let me check it tmrw. Curently waiting for my flight from LAX to Detroit

[ClaytonJY]

Okay so that makes sense, but all statements in a process are always run together, correct? Seems like a better behavior would be to return a list() of the results if there's more than one.

[javadch]

The b and b1 is weird i have to debug the grammar to see what is going on.

[javadch]

It takes time to convert all, while in many cases not all of them are needed e.g., intermediate results. I suggest to have a function to return only the names of the variables of a process, so that users know what is available to ask for its actual data.

[ClaytonJY]

Is it not possible to specify in the grammar that not all statements need to return their results when the process is finished? It seems odd to make users go through an extra step of fetching their results from Java.

[javadch]

lets fix the blocking issues first, then we can come back to discuss these enhancements. The R's DBI works in the same way, but for single statements. set connection, impose query, execute query and fetch result.

[javadch]

I made some minor changes in the test.R and also in the getEngine function. The jar changes are just to be sure everything is up to date (the changes may not be relevant to this issue). Using this version the test demo runs for me. if it still fails on your machine, please:

• delete the log folder
• load and built the package
• run the demo
• send me the log file
• send me the location of the installed package

[javadch]

Tested and passed on both Windows and Linux. Used the latest develop version as of today Sept. 15