-
Notifications
You must be signed in to change notification settings - Fork 33
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
Added classes Connect.Call and StoredProcedureOutcome #53
Conversation
@amihaiemil Many thanks for the PR, let me find a reviewer for it |
@mkordas review this one please |
@amihaiemil I'll do a review here today |
@mkordas great, thanks :D |
@amihaiemil sorry, all in all I've missed it yesterday |
@@ -2,3 +2,8 @@ target/ | |||
.idea/ | |||
*.iml | |||
.DS_Store | |||
/bin/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amihaiemil do we need this first slash here?
@amihaiemil see my initial review above |
@@ -309,6 +310,25 @@ public void commit() throws SQLException { | |||
} | |||
|
|||
/** | |||
* Call an SQL stored procedure. | |||
* | |||
* <p>JDBC connection is opened and, optionally, closed by this method. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amihaiemil does this method really close connection? where?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mkordas the run
method does that in the finally
block. All the other methods, e.g. execute()
say that in the javadoc, so I left it there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amihaiemil OK, let it be. But I still does not get optionally
concept, can you explain?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mkordas Yes. In run
's finally
block the statement
is closed, not the connection (I made a mistake in the first comment). optionally
means if it's autocommit
or not... "closing" the connection actually means "commiting" it. And autocommit is dictated by class boolean this.auto
which can be set with the autocommit(boolean)
method. The Javadoc actually has in regards the fluency of the API, that's why it seems a bit hard to understand, in my opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amihaiemil right, so can you put the above information to Javadoc? It's more accurate than saying connection is opened and, optionally, closed
@yegor256 Can we merge here? It's also very old, catching spiders already :)) |
@rultor try to merge |
@amihaiemil @yegor256 Oops, I failed. You can see the full log here (spent 7min)
|
@mkordas the coveralls repotoken has to be specified... I guess in Maybe I should make the coveralls plugin execute only by direct command |
@yegor256 I also tried to specify Please answer my question above. |
@amihaiemil It's working like that for other projects, see e.g. https://github.com/jcabi/jcabi-s3/blob/6ca906085702054f005ab1b2fde15b19ae80527d/pom.xml#L182 |
@yegor256 can you please check whether |
@yegor256 ping |
@yegor256 can you please check whether ${coveralls.token.jdbc} points to the proper token on Rultor? |
@yegor256 ping |
@rultor merge |
@amihaiemil @yegor256 Oops, I failed. You can see the full log here (spent 4min)
|
@yegor256 can u check the token specified a few comments up? |
@rultor try to merge again |
@amihaiemil @yegor256 Oops, I failed. You can see the full log here (spent 5min)
|
@yegor256 still the same, looks like |
@rultor try to merge |
@rultor deploy now pls |
PR for #51 .
Added Outcome and Connect that provide calling stored procedure functionality.
E.g 1.
Say you have a stored procedure which takes 1 INPUT and 1 OUTPUT, this is how you would use the API to call it:
Unlike
execute()
and others,JdbcSession.call(Outcome)
opens aCallableStatement
, instead of aPreparedStatement
. Output parameters need to be registered by providing aPreparation
.E.g. 2:
If you have IN and OUT Params together, let's say in the following order:
IN, OUT, IN, OUT
, then you have to use theOutcome
as follows:new StoredProcedureOutcome<Object[]>(2, 4)