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

New Release #92

Closed
christianjgreen opened this issue Aug 14, 2017 · 21 comments
Closed

New Release #92

christianjgreen opened this issue Aug 14, 2017 · 21 comments

Comments

@christianjgreen
Copy link

Is there an ETA on the latest release from the master branch? The new error handling for connections {:error, msg} is needed for a DB driver using this repo!

@christianjgreen
Copy link
Author

@ankhers

@michalmuskala
Copy link
Member

The new release is planned after postgrex and mariaex get updated, so we know the approach is correct.

@christianjgreen
Copy link
Author

Is there a version you guys are looking for? Postgrex 12 days ago and Mariaex 3 days ago

@ankhers
Copy link

ankhers commented Aug 14, 2017

I believe the work for Postgrex is (mostly?) complete. I was asked to do the same for Mariaex, but my unfamiliarity with the database and codebase prevented me from getting it done.

I do not mind taking another crack at it. Unfortunately, even if I thought I got it working, my unfamiliarity with things would prevent me from knowing for sure.

@michalmuskala
Copy link
Member

The work is being done in elixir-ecto/postgrex#321 and xerions/mariaex#196

@fishcakez
Copy link
Member

@ArThien is the driver open source? If so can you link to an issue that we can track? It's possible we could look at a v1.1 branch whose parent commit is as far back as possible - before new features were introduced.

@ankhers
Copy link

ankhers commented Aug 19, 2017

@ArThien is talking about elixir-mongo/mongodb#145. I would prefer to have the change that returns {:error, exception} instead of raising an exception when you try to query a disconnected connection.

If this is going to take a while though, I could look into rescuing instead of waiting on this.

@christianjgreen
Copy link
Author

@ankhers Would you mind using rescue ! for now over tuples?

@fishcakez
Copy link
Member

Unfortunately the latest commit we can release a patch version for is 53271f7. I think there are still cases after that where DBConnection can raise instead of returning an error. I think all these are changed at some point so it might be we can create a v1.1 branch and you can add any further of these fixes required. It could be easily to rescue.

Unless limited to the DBConnection.Sojourn pool there are still other cases in master where an exit/1 can happen as part of anticipated operation so I think would always need catch exits also. Given that I believe this driver uses DBConnection for the ownership pool it will is likely required.

@ankhers
Copy link

ankhers commented Aug 21, 2017

I will test a version of mongodb with rescuing. I will look into doing this within the next couple days.

@christianjgreen
Copy link
Author

@ankhers our dev team thanks you :)

@leifg
Copy link

leifg commented Sep 19, 2017

Is there anything I can do to help? I would also be very interested in a new release.

@fishcakez
Copy link
Member

We would need to get things working in Ecto.

@shdblowers @scouten are mssql and sqlite adapters about to detect transaction status (whether inside or outside a transaction) without a roundtrip across network? Obviously for SQLite it is just whether we can detect it as we're on same host.

IIRC neither of those adapter supports streaming so we don't need to worry about the other changes.

@shdblowers
Copy link

@fishcakez is that the work for supporting Ecto 2.2?

@josevalim
Copy link
Member

@shdblowers not necessarily. Once we release a new db_connection version, I can backport any necessary fix to both Ecto 2.1 and Ecto 2.2 branches. So hopefully you are able to track those in isolation. The work I believe @fishcakez was referring to was to update to db_connection master.

@shdblowers
Copy link

Ah yes sorry, got confused between the two streams of work.

mssqlex is ready for the update to DBConnection.

@fishcakez
Copy link
Member

fishcakez commented Oct 2, 2017

@shdblowers the mssqlex PR is using the client, rather than the database, to set the transaction status: https://github.com/findmypast-oss/mssqlex/blob/9cee8944fa38f92cc2e000048fff232b8f5a1f75/lib/mssqlex/protocol.ex#L272. If ODBC doesn't (or can't) send the transaction status as part of the response then we shouldn't implement the feature because the client can get out of sync with the database if a user does query(.., "BEGIN...).

@shdblowers
Copy link

Hi @fishcakez , with ODBC transactions are handled at the connection level.

You can either be connected in auto-commit (each statement committed when sent) or manual-commit mode (everything is a transaction).

In that way the client and DB stay in sync with the status of a transaction.

However, ODBC does not send the transaction status as part of the response.

@fishcakez
Copy link
Member

fishcakez commented Oct 10, 2017 via email

@shdblowers
Copy link

No, when connected in manual commit mode you can commit or rollback a transaction by calling :odbc.commit, not with a raw query.

@fishcakez
Copy link
Member

Thank you @shdblowers, sounds good! Lets move forward with a release once the milestone issues are handled.

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

7 participants