search

foursquare / fsqio

A monorepo that holds all of Foursquare's opensource projects
Apache License 2.0
252 stars 54 forks source link

Scala 2.12 branch #37

Open stuhood opened 7 years ago

stuhood commented 7 years ago

Twitter consumes some of the twofishes libraries, and are interested in helping to get a 2.12 branch started. This is a notice of our intent to do so!

mateor commented 7 years ago

👋

Yeah...one of the drawbacks of the Fsq.io mode is that it can chain users to our internal versions. It most often is a problem for our Pants modules, actually.

The state of fsqio master is directly split off of our monorepo so in order to convert Fsqio/master we would have to upgrade our internal codebase. Seeing as that is not imminent, I am okay with running a publish from a 2.12 branch of Fsq.io.

One open question is that the io.fsq publishing model is to publish all jars as a unit - we are essentially versioning the repo itself (most recently 1.32). We could be willing to compromise on that as needed but have a strong preference of maintaining that practice.

I am going to ping a few coworkers with domain knowledge (You already spoke with @jvandew and @tdyas is Tom Dyas who tagged along to the last Pants summit).

jvandew commented 7 years ago

Twofishes at least will have issues with the Salat library it uses. There are apparently plans for a 2.12 release branch there, but no real timeline: https://github.com/salat/salat/issues/183. We do already have a pull request open with them that might address the issues raised in that issue though, so possibly we could just build and host a 2.12 jar of that patch in the meantime.

Long term I would really like to see us rewrite the mongo layer of the indexer to use Rogue. It's pretty disparate from the rest of our stack as is and I think https://github.com/foursquare/fsqio/issues/33 has shown that the current state is not maintainable going forward. I don't think that would be too tricky to make happen, just a matter of someone finding the time for it.

stuhood commented 7 years ago

The first things I encountered while attempting the upgrade:

(unrelated: it seems like the upkeep script might have a dependency on binary builds from somewhere, as it fails to create a virtualenv ... I was able to get past that by replacing the pants script in the repo with the one from www.pantsbuild.org/install.html)

mateor commented 7 years ago

Hey Stu.

Let's see - so I think we have a solution for salat- I have been holding off pushing an fsqio update because of some internal churn. But we have inlined the one class from Salat we use, I can push that out soon (better would have been to consume the shading rules, but we didn't get there).

The clapper jar looks like something we can rip out. It is just an arg parser used by spindle - any more modern arg parser would do just as well.

I would be interested in seeing the upkeep failure - the Travis build passed and I know we have people using this repo. But I could believe that there is a lingering failure state still in there somewhere.

mateor commented 7 years ago

@tdyas and @jvandew have closed these issues internally. I will update fsqio sometime before next Monday.

stuhood commented 7 years ago

Huzzah! Sorry that I was so useless here.

mateor commented 7 years ago

So, I was wrong about this being done - I apologize Stu : (

We had inlined a class from Salat in order to upgrade our driver - I thought that meant we were finished with the dependency. But that was inaccurate - there are plenty of Salat callsites in twofishes itself.

Salat is a serialization library and this is the only project in our repo that uses it. Internally, our solution would be to port it to Rogue. I am considering how we could make that happen, but it is obviously a bigger job than just pushing out an Fsq.io update.

In the meantime, I do not have a good answer for you here. We could consider just trying to publish a shaded salat, or expand our inlined fork.

landism commented 7 years ago

salat 1.11.1 has been published for 2.12: http://search.maven.org/#search%7Cga%7C1%7Csalat

mateor commented 7 years ago

Wow, thanks. Jacob just linked me to this, I missed that comment! We can probably get move to that for now, then. Let me talk with some folks and I will follow up.

stuhood commented 6 years ago

How's this going?

tdyas commented 6 years ago

cc @jvandew and @mateor

mateor commented 6 years ago

Hi - we asked @wcauchois to take this up.

wcauchois commented 6 years ago

Hey, just to update on this, I ended up ripping out Salat and porting the twofishes index builder code to use Rogue (kind of the nuclear option, but also ended up making some code nicer :). That PR is merged internally and we should be able to publish it soon!

stuhood commented 6 years ago

Awesome... thanks @wcauchois!

mateor commented 6 years ago

Hi - this had the unlucky timing to land in the middle of our converting Fsq.io (and our internal repo) from pom-resolve to Ivy.

But that update has been shipped. The 2.0.2 jars for twofishes have been published for Scala 2.11. We have internal blockers to 2.12 still, but the jar publish is now using standard Pants mechanisms and you should be able to interact with them using common methods

mateor commented 6 years ago

Current 2.11 jars are 2.02 and above

stuhood commented 6 years ago

Hey Mateo! Have you had any luck with your migration?

mateor commented 6 years ago

The last requirement for upgrading Fsq.io to Scala 2.12 is Finagle upgrade, but that is blocked internally by our web team. They are doing that work this quarter but I can't promise when that will happen.

But I did knock over the 2.12 blockers for fsq.io as you identified them back in the original issue. The Twofishes and Spindle library has no blockers to Scala 2.12. As a bonus, the resolve and publish mechanisms have been ported off pom-resolve and should be ~roughly familiar to upstream Pants.

mateor commented 6 years ago

I can try and configure a ~working resolve and post what the migration for twofishes alone might look like

stuhood commented 6 years ago

@mateor : Thanks Mateo! If those issues are resolved and the repo is on a more recent pants version with fewer plugins, then I think I might be unblocked already. Will take a look soon.

mateor commented 6 years ago

Yeah, as a part of this project I deprecated pom-resolve and pom-publish, so this was a slow burn for sure.

We did make a 2.12 branch and discovered two additional issues. One was our own foursquare.country-revgeo jar, which was being published by david blackman from a separate repo, presumably on your behalf :)

I moved that jar into Fsq.io, bootstrapping the shapefiles at runtime: https://github.com/foursquare/fsqio/commit/646692d919444ad20a3a5752254c065d97d03b3c

The other issue is another dependency that is not published for 2.12, jerkson. We have assigned @iantabolt to that issue and he is attacking it this sprint.

mateor commented 6 years ago

I believe stu is on vacation now, lets try and close this issue in April

mosesn commented 6 years ago

@mateor what's the current status on this? I work with @stuhood

mateor commented 6 years ago

Hi @mosesn. We made progress, by virtue of small cuts. I think you are probably familiar with our core issue, which is Finagle upgrade, specifically in our internal web framework. Migrating over to httpx and back has been a war of attrition.

I actually spent most of today preparing an Fsq.io update that removes jerkson (thanks @iantabolt ), which was the current blocking issue. That update should be mostly ready to go. When it comes out I will check the 2.12 branch I have stashed and see what we see.

mosesn commented 6 years ago

OK, I'm happy to answer any questions you have about finagle. I also moved back to NYC, so I could swing by and chat some day if you want.

mateor commented 6 years ago

Hey welcome back - I just may try and take you up on that :)

I was able to update Fsq.io last night - the big contribution towards this issue was by Ian Tabolt , removing jerkson.

I have a dummy fork of 2.12 fsq.io I test resolution with as we make progress. I think we are down to one last dependency we need to upgrade/remove. Here is my commit message from that branch:

    Updated resolution for Scala 2.12

    Only one left - for the resolve, that is. We will still
    need to debug whatever compile issues we find afterwards

                                ::::::::::::::::::::::::::::::::::::::::::::::

                                ::          UNRESOLVED DEPENDENCIES         ::

                                ::::::::::::::::::::::::::::::::::::::::::::::

                                :: com.rockymadden.stringmetric#stringmetric-core_2.12;0.27.4: not found

                                ::::::::::::::::::::::::::::::::::::::::::::::

```
Surprise surprise, it is used in the geocoder

```
mateo-2:fsqio mateo$ git grep rockymadden
3rdparty/BUILD.opensource:  name = 'rockymadden',
3rdparty/BUILD.opensource:    scala_jar(org = 'com.rockymadden.stringmetric', name = 'stringmetric-core', rev = '0.27.4')
src/jvm/io/fsq/twofishes/indexer/importers/geonames/BUILD:    '3rdparty:rockymadden',
src/jvm/io/fsq/twofishes/indexer/importers/geonames/PolygonLoader.scala:import com.rockymadden.stringmetric.similarity.JaroWinklerMetric
src/jvm/io/fsq/twofishes/indexer/importers/geonames/PolygonLoader.scala:import com.rockymadden.stringmetric.transform._
src/jvm/io/fsq/twofishes/indexer/scalding/BUILD:    '3rdparty:rockymadden',
src/jvm/io/fsq/twofishes/indexer/scalding/BaseUnmatchedPolygonFeatureMatchingIntermediateJob.scala:import com.rockymadden.stringmetric.similarity.JaroWinklerMetric
src/jvm/io/fsq/twofishes/indexer/scalding/BaseUnmatchedPolygonFeatureMatchingIntermediateJob.scala:import com.rockymadden.stringmetric.transform._
src/python/fsqio/pants/buildgen/jvm/scala/third_party_map_jvm.py:    'rockymadden': 'rockymadden',
```


This is core geocoder code so possibly not so fun to rip out. I will again lean on @iantabolt, who was on our venues team before he transitioned to infra.

I was going to have him pick up our finagle upgrade next sprint - I will ask if he can tackle this issue as a prerequisite.
stuhood commented 6 years ago

@mateor : Awesome work! Glad to see that you guys are so close.

I was able to hack around that last dep in order to do a private publish of twofishes-countryinfo, so as far as I can tell we're unblocked over here. Sorry that we were not able to do more to help with the upgrade, but very very happy to see the fsqio build working out of the box. Exciting times!

mateor commented 6 years ago

Man...I am so glad to hear that. I had self-assigned it but was scrambling to find the time.

I was glad to make progress towards Scala 2.12 but realistically that has a ways to go still. It took significant resources to convert to 2.11 in 2017 and we expect to need two independent Finagle upgrades to safely migrate our internal web stack for 2.12. But even without getting to a full 2.12 upgrade yet, I am relieved our steady efforts got you unblocked.

Did you just publish the 2.12 fork of rockymadden? I would be pretty interested in looking at your 2.12 patch, if you can share it

The various tickets this issue spawned inspired a near overhaul of our build stack, including deprecating pom-resolve, vexsrc and pom-publish in favor of upstream Pants tasks. Some public thanks to the many folks who volunteered their cycles pushing this rock up the mountain.

From: @jvandew

From: @wcauchois

From @mateor:

From: @tdyas

From: @TansyArron

From @iantabolt

StephanSchmidt commented 6 years ago

Sorry to be bothersome, some updates after 3months? :-)

mateor commented 6 years ago

Twitter said they had what they needed. What do you need?

StephanSchmidt commented 5 years ago

As this project is dead, could some close it please? People might accidentally use it.

jvandew commented 5 years ago

This scala 2.12 issue? We are still working on our 2.12 migration internally and will get that work pushed here once we're upgraded.

If you need 2.12 artifacts published in the meantime we can likely unblock you there, but I will refer you back to mateor's question from last August: what do you need?

StephanSchmidt commented 5 years ago

Ok, I need to run Rogue with 2.12, sorry didn't understand the question correctly the last time.