Skip to content
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

Missing suspendables in comsat-jdbi #84

Open
victornoel opened this issue Nov 15, 2016 · 8 comments
Open

Missing suspendables in comsat-jdbi #84

victornoel opened this issue Nov 15, 2016 · 8 comments

Comments

@victornoel
Copy link
Contributor

Hi,

Apparently the following suspendables is missing in comsat-jdbi:

org.skife.jdbi.v2.sqlobject.SqlObject$3.intercept

Or actually, I suspect that the following suspendables:

org.skife.jdbi.v2.sqlobject.SqlObject$1.intercept
org.skife.jdbi.v2.sqlobject.SqlObject$2.intercept

Should be:

org.skife.jdbi.v2.sqlobject.SqlObject$2.intercept
org.skife.jdbi.v2.sqlobject.SqlObject$3.intercept

As for $1, I don't know if it needs to be marked as suspendable, it is possible, but then it should be:

org.skife.jdbi.v2.sqlobject.SqlObject$1.accept
@victornoel
Copy link
Contributor Author

I am using jdbi 2.73, but it is the same with 2.72, the version referenced in comsat-jdbi…

@victornoel
Copy link
Contributor Author

Actually, I think there is a lot more missing…

Some are related to all the SQL Object API way of using jdbi… it's a mess :)

@victornoel
Copy link
Contributor Author

Here are some others:

org.skife.jdbi.v2.GeneratedKeys.org.skife.jdbi.v2.GeneratedKeys(org.skife.jdbi.v2.tweak.ResultSetMapper,org.skife.jdbi.v2.SQLStatement,java.sql.Statement,org.skife.jdbi.v2.StatementContext,org.skife.jdbi.v2.ContainerFactoryRegistry) (GeneratedKeys.java:57)
org.skife.jdbi.v2.Update$2.munge (Update.java:86)
org.skife.jdbi.v2.util.LongColumnMapper.mapColumn(java.sql.ResultSet,int,org.skife.jdbi.v2.StatementContext) (LongColumnMapper.java:34)
org.skife.jdbi.v2.util.LongColumnMapper.mapColumn (LongColumnMapper.java:22)
org.skife.jdbi.v2.sqlobject.FigureItOutResultSetMapper.map (FigureItOutResultSetMapper.java:35)

@pron
Copy link
Contributor

pron commented Nov 15, 2016

A PR would be welcome.

@victornoel
Copy link
Contributor Author

Once I am exactly clear on the problem, I would love to :) For example I don't exactly understand how all of this goes hand in hand with the JdbiClassifier

See also my post on the mailing list which details the context of these issues: https://groups.google.com/forum/#!topic/comsat-user/j4oIEtxHW0I

@pron
Copy link
Contributor

pron commented Nov 16, 2016

For example I don't exactly understand how all of this goes hand in hand with the JdbiClassifier…

The classifier is a programmable way of marking suspendable methods. If we're dealing with generated methods (that don't always have the same class name), the classifier would be a good place to add them.

@circlespainter
Copy link
Member

@victornoel Could you also add corresponding test cases when you work on the PR? Thanks, expanding test (and use case) coverage is always a great thing.

@victornoel
Copy link
Contributor Author

@circlespainter sorry, but I'm not sure I will be working on that for now (but if/when I do, I will add tests yes!), because it's too much hassle to make it works and the logs are filled with garbage (I'm not sure why, see the mailing list issue).
For the record, I ended up using runBlocking with my own ExecutorService to call jdbi: this ends up doing the same as using the comsat jdbi integration without having to manage the suspendables, since comsat jdbi is based on comsat jdbc which is based on runBlocking.

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

No branches or pull requests

3 participants