Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    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
    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
    Yarek T
    @YarekTyshchenko

    This test should demonstrate the problem:

          "Seq of tuple params for IN operator" in {
            val p1 = Seq(("foo", "bar"), ("three", "four"))
            expectQuery("SELECT * FROM foo WHERE (a, b) IN (($1, $2), ($3, $4))", "foo", "bar", "three", "four") {
              sql"SELECT * FROM foo WHERE a IN ($p1)"
            }
          }

    Its a bit of a weird syntax, but postgres does support it

    @jeremyrsmith ^^.
    Philippe Laflamme
    @plaflamme
    Hi, sorry for the noise, but I'm looking for feedback on finagle/finagle-postgres#128 I'd just like to know if it's something that will be considered for merging before adding more commits...
    Philippe Laflamme
    @plaflamme
    Bump ^
    jeremyrsmith
    @jeremyrsmith
    @plaflamme I think it’s a good direction. I’d merge it if it were fleshed out a bit; I’d just try to minimize API breakage if possible.
    Philippe Laflamme
    @plaflamme
    Great, thanks for the feedback! It's been a while, so what parts of it would need to be more "fleshed out"? Happy to do those first. I'll see what I can do for the API breakage...
    Philippe Laflamme
    @plaflamme
    @jeremyrsmith I've added the remaining bits and also undone the API breakage. Please take a look finagle/finagle-postgres#128
    Philippe Laflamme
    @plaflamme
    @dangerousben @leonmaia Any chance we can get a 0.12.0 release with the latest changes (https://github.com/finagle/finagle-postgres/compare/v0.11.0...master)?
    Ben Spencer
    @dangerousben
    I don't have access to cut a release (afaik), if @leonmaia's not around then @vkostyukov might
    jeremyrsmith
    @jeremyrsmith
    I have access but TBH I’m not sure how it works anymore. I used to basically just sbt +publish but it uses a more complicated (undoubtedly better) process now. Maybe linked to github releases?
    Leon Maia
    @leonmaia
    For generating a new release you just need to update version.sbt with the new release number removing the snapshot tag. The rest is done automagically :)
    Sorry - I'm not using postgres at work, moved to mysql and been very busy for the last year working with Kafka. So I'm not with the time to be 'very' active here.
    I'll merge the update PRs from scala-stewart and tag a new release.
    Leon Maia
    @leonmaia
    v0.12.0 has been published @plaflamme, thanks a lot for the contribution! Release notes: https://github.com/finagle/finagle-postgres/releases/tag/v0.12.0
    Philippe Laflamme
    @plaflamme
    Thanks so much! BTW: the Kafka work you are referring to, is that related to Finagle? Being curious, because we have a finagle-kafka client and are wondering if people are willing to contirbute
    Leon Maia
    @leonmaia
    I'll DM you so we don't pollute here too much with off-topic :)
    Ben Spencer
    @dangerousben
    thanks @leonmaia! (and @plaflamme for the code)
    jilen
    @jilen
    Hi, can someone have a look at finagle/finagle-postgres#163
    Ben Spencer
    @dangerousben
    I'm going to close some of the old scala-steward PRs and (unless anyone objects) merge the newer ones that pass
    and then hopefully get around to reviewing some of the outstanding human PRs, including the above
    does anyone know if scala-steward requires your dependencies to be specified in the most simple way possible, rather than (eg) having a val finagleVersion that you apply later elsewhere?
    I assume it it probably does, but can't really find any documentation to the effect
    Ben Spencer
    @dangerousben
    @jilen copying in the whole of patchless seems pretty extreme, can't it be updated upstream?
    Ben Spencer
    @dangerousben
    interesting note: the client is configured to retry on timeout, so if you were using a TimeoutFilter and expecting the exception to bubble up to the top, it er, won't
    Ben Spencer
    @dangerousben
    concerning addendum to note: it appears that if the retry thing happens, connections are permanently lost from the pool (or rather, the pool thinks it has them, but it doesn't according to ChannelStatsHandler)
    Ben Spencer
    @dangerousben
    looks like random build (travis) build failures caused by certain generated json are not an uncommon occurrence, can anyone confirm that?
    Ben Spencer
    @dangerousben
    ah, but it would seem now I've caused a non-random build failure by upgrading sbt-pgp
    ontologiae
    @ontologiae
    Hi, i would to be sure that fnagle-postgres work with Scala 2_13
    I have this error : unresolved dependency: io.github.finagle#finagle-postgres_2.13;0.4.0-SNAPSHOT: not found
    Ben Spencer
    @dangerousben
    @ontologiae I believe the currently released version (0.12.0) doesn't, but there is support in master
    also 0.4.0-SNAPSHOT is ancient, looks like we need to update the guide
    @leonmaia if you have the time could be worth cutting a release that includes 2.13 support?
    Philippe Laflamme
    @plaflamme
    Any objections to switching the use of a travis-provided PgSQL to an embedded one? This is an example finagle/finagle-postgres#186 of converting a test to rely on the embedded version. It makes running tests entirely self contained...
    jeremyrsmith
    @jeremyrsmith
    I guess my only concern would be, how is the embedded one implemented? If it's not actual postgresql, then it doesn't seem right to run the tests against it
    Philippe Laflamme
    @plaflamme
    Yeah, sorry I should have mentioned. This is the actual postgresql binary embedded in a jar. At runtime, it extracts the correct one for the platform and starts it pointing a temporary directory. The main drawbacks are: harder to change the postgres version since that's dictated by the test dependency and potential runtime issues when extracting / running. But the upside is pretty big IMO since you can just do sbt test and things "just work".
    jeremyrsmith
    @jeremyrsmith
    Hmm, so the upside is that it eliminates some one-time setup for contributors to run the integration tests (but if you're contributing you probably have postgres installed locally anyway right)? And the downside is that it makes it impossible for CI to run integration tests against different versions of postgres (which AFAIK isn't something finagle-postgres does anyway). So I guess I'm ambivalent about it :grinning:
    Philippe Laflamme
    @plaflamme
    Well, I suppose it's about being self-contained. With this approach, there is no need to assume that a contributor has pgsql running anywhere (nor that their environment variables are setup correctly). In fact, this assumption had to be clarified recently: finagle/finagle-postgres#157 For supporting multiple versions, it's solvable and I'm willing to take a stab at it if this is something that has a chance of being merged. In fact, I'll submit an example in the existing PR. The ultimate upside would be that sbt it:test locally behaves the same as within CI with no additional setup as well as test against multiple pgsql versions.
    Philippe Laflamme
    @plaflamme
    @jeremyrsmith I've updated the PR to use a different embedded pgsql (a fork from the first one) that makes multi-version testing easier. The PR uses one sbt project per pgsql major version and runs the IntegrationSpec against that, there are alternate approaches I suspect, but this one works without too much trouble. The end result is testing against 9, 10 and 11 locally simply through sbt test.
    Philippe Laflamme
    @plaflamme
    Also, FYI finagle/finagle-postgres#187 some fun with timezones.
    Ben Spencer
    @dangerousben
    oh nice, I hit that timezone bug occasionally
    would be interested to know whether we can support TLS with an embedded pg, the current CI build doesn't test it
    Ben Spencer
    @dangerousben
    speaking of TLS... it appears to be broken again
    Ben Spencer
    @dangerousben
    @plaflamme are you around? I'm trying to debug these intermittent travis failures and it's possible it has something to do with your changes to ValuesSpec in #187 (which I have merged, breaking the build in the process)
    Ben Spencer
    @dangerousben
    hmm, this might all be fixable by finally doing this TODO anyway: //TODO: change this once prepared statements are available
    Ben Spencer
    @dangerousben