Where communities thrive

Join over 1.5M+ people
Join over 100K+ communities
Free without limits
Create your own community

Explore more communities

finagle/finagle-postgres

PostgreSQL protocol support for Finagle

People

Repo info

Activity

Leon Maia

@leonmaia

yeah, should be there soon.

Ben Spencer

@dangerousben

I'm thinking maybe we don't need a 1:1 mapping between CommandCompleteStatuss and actual postgres responses

eg is there really any benefit to having separate case objects here? https://github.com/finagle/finagle-postgres/blob/master/src/main/scala/com/twitter/finagle/postgres/connection/ConnectionStateMachine.scala#L91

Leon Maia

@leonmaia

I believe the only benefit is to have separate case classes assigned to each tag https://github.com/finagle/finagle-postgres/blob/3d4ab10b82b37503fb61b0fd128fa7a7b176d1da/src/main/scala/com/twitter/finagle/postgres/messages/BackendMessageParser.scala#L221

btw, I started the release of 0.8.2

I tried to update the finagle version but it breakes with the latest version.

Leon Maia

@leonmaia

I'll try to work on this as soon as I can, I changed jobs recently and free time is not something that you actually have during the first 100 days :D

Leon Maia

@leonmaia

Release 0.8.2 out, with both the SSL session fix and the name resolution fix.

Ben Spencer

@dangerousben

is RowDecoder ever intended to be implemented manually or should you just not use queryAs if you want custom decoding?

re: case classes per tag, is that actually a benefit though? I suppose I can envision some future use for metrics purposes or something but for now it just seems to be boilerplate

Leon Maia

@leonmaia

In my understanding now it just seems "boilerplate", I guess it adds a little bit of more clarity 🙂, but probably removing it makes the code more simple to read though.

Ben Spencer

@dangerousben

wrt RowDecoder, the fact that queryAs is declared as queryAs[T <: Product] leads me to suspect that it was only ever intended to be used with auto derivation from case classes

we were going to write a custom RowDecoder for a trait that picked the correct subclass based on the data, but it's not really any different to writing a reusable Row => T function to pass to prepareAndQuery

@leonmaia I was more concerned with ease of adding support for new tags than readability

currently you have to touch three places in the code like this: finagle/finagle-postgres@dbdb5fa

jeremyrsmith

@jeremyrsmith

@dangerousben IIRC I think RowDecoder was intended to support auto-derivation… but it’s possible there could be APIs for manually constructing it that are easier than dealing with Row directly

i.e. with lens-ish things and scodec-type combinators

Ben Spencer

@dangerousben

what's supposed to happen if a ValueDecoder throws an exception?

from the code it looks rather like it will silently return null :|

jeremyrsmith

@jeremyrsmith

ValueDecoder returns a Try… IIRC there are places where the error is silently discarded (and you get null or 0) but there should be parallel APIs that give you the Try. My fault, sorry – maybe it should throw there, but it would have been a pretty big behavior change I think

i.e. https://github.com/finagle/finagle-postgres/blob/master/src/main/scala/com/twitter/finagle/postgres/Responses.scala#L84 (there is a doc comment not to use it if you care about keeping failures… not really good enough I know, but I didn’t want to introduce exception-throwing behavior into existing Row APIs)

Ben Spencer

@dangerousben

does anyone know what might cause this:

WARNING: channelOpen() called when channel was already open, clobbering an existing timer

Ben Spencer

@dangerousben

hmm might be because my integration tests are creating multiple clients with the same details and not cleaning them up

Stefan Dietiker

@d-s-d

If I await the result of a Future (e.g., Await.result(future, Duration(10, TimeUnit.SECONDS))) and run into a time-out, is the underlying database-transaction aborted?

Ben Spencer

@dangerousben

I think probably not, but I'd test it to be sure

jilen

@jilen

Is finagle postgres still maintained ?

Vladimir Kostyukov

@vkostyukov

@jilen Yea, @leonmaia is the current maintainer. @jeremyrsmith (previous maintainer) and myself contribute ocasionally.

Leon Maia

@leonmaia

Master build green again... for some reason travis failed to build when I merged the lib upgrades, but now it's all green again :green_apple:

Ben Spencer

@dangerousben

any plans for a release in the near future? would quite like to upgrade to finagle 19.1.0

Leon Maia

@leonmaia

I'll do one today :+1:

Ben Spencer

@dangerousben

nice one, thanks :)

Leon Maia

@leonmaia

0.8.3 should be live soon @dangerousben , couldn't do it yesterday my wife wouldn't let me open my computer during valentines :laughing:

Ben Spencer

@dangerousben

haha :)

thanks

Ben Spencer

@dangerousben

@leonmaia looks like finagle-postgres-shapeless didn't go out, this often seems to happen so I'm guessing it's missing from the release script or something?

Leon Maia

@leonmaia

The issue was that shapeless was not aggregated to the core project - this is fixed in my PR. I'll go ahead and merge it so we have this properly working.

Ben Spencer

@dangerousben

thanks again :)

Leon Maia

@leonmaia

no worries :)

Leon Maia

@leonmaia

ok, it worked as expected. @dangerousben I'll trigger a release 0.9.0now and you should now have both artifacts.

Ben Spencer

@dangerousben

great

darri89

@darri89

Hey guys, any plans for an ugprade to finagle 19.2.0? It appears they did some breaking API changes in that

Vladimir Kostyukov

@vkostyukov

@darri89 Looks your right; finagle/finagle-postgres#117 is failng.

Leon Maia

@leonmaia

Hi @darri89 thanks for the heads up. I'll have a look on it.

Leon Maia

@leonmaia

@darri89 I got a green build with 19.2.0, I'll be doing a new release tomorrow.

darri89

@darri89

@leonmaia great thanks!

Yarek T

@YarekTyshchenko

Hi. I'm trying to write a query that uses a WHERE (a, b) IN ((1, 2), (3, 4)) style of where clause. It looks like it's not possible to pass a Seq[T] as a param. Is there any way around this?

Yarek T

@YarekTyshchenko

Interestingly using sql"<query>" syntax works when passing a list of a type fine, but doesn't work with a Tuple, or a case class that has the two fields

jeremyrsmith

@jeremyrsmith

@YarekTyshchenko it looks like tuples should work: https://github.com/finagle/finagle-postgres/blob/master/finagle-postgres-shapeless/src/test/scala/com/twitter/finagle/postgres/generic/QuerySpec.scala#L92

can you post your query and the error(s) you get? Sorry, it’s been a while since I wrote that stuff and it’s not well documented :( There are some quirks of the query DSL that you can hopefully see in that spec