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

Add client cache sub project #337

Merged
merged 1 commit into from Jul 9, 2018
Merged

Add client cache sub project #337

merged 1 commit into from Jul 9, 2018

Conversation

peterneyens
Copy link
Contributor

Add a cache for RPC clients.

build.sbt Outdated
.in(file("modules/client-cache"))
.settings(moduleName := "frees-rpc-client-cache")
.settings(clientCacheSettings)
.settings(addCompilerPlugin(%%("paradise") cross CrossVersion.full))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this line I get a Missing required plugin: macroparadise compiler error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was overriding the libraryDependencies before, but it looks like we only get the paradise plugin and a scalatest test dependency from the project plugin.
I switched back to ++=, so should be fine now.

lazy val clientCacheSettings: Seq[Def.Setting[_]] = Seq(
libraryDependencies := Seq(
%%("log4s", V.log4s),
%%("fs2-core", "0.10.1"),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This it the fs2 version used by the fs2-reactive-streams version that we are currently using.
This one is sadly still on cats-effect 0.8.0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to 0.10.5, also added an explicit dependency to fs2 0.10.5 to internal so that the transitive fs2 0.10.1 gets evicted.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks @peterneyens

@@ -73,6 +73,13 @@ lazy val `client-okhttp` = project
.settings(clientOkHttpSettings)
.disablePlugins(ScriptedPlugin)

lazy val `client-cache` = project
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add it to the "modules registry" below in this file

lazy val clientCacheSettings: Seq[Def.Setting[_]] = Seq(
libraryDependencies := Seq(
%%("log4s", V.log4s),
%%("fs2-core", "0.10.1"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add the 0.10.1 fs2 val with the rest, same for the better-monadic-for

(map, _) <- ref.get
client <- map
.get(hostAndPort)
.fold {
Copy link
Contributor

@diesalbla diesalbla Jul 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, I prefer using fold. However, once once each case starts spawning over three-four lines, I find it more readable to use a match:

client <- map.get(hostAndPort) match {
  case None =>  
      /** 6 lines of code*/
  case Some(clientMeta) =>
     /** 5 lines of code*/
} 

I find indentation an easier separator than several-line-separated parentheses.

ref
.modify(_.leftMap(_.updated(hostAndPort, clientMeta.copy(lastAccessed = now))))
.as(clientMeta.client))
(_, lastClean) <- ref.get
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we extract lines 82-85 as a separate def cleanIfStale(atTime: Instant): F[Unit] method?

@peterneyens peterneyens merged commit f96cf1f into release/0.14.0 Jul 9, 2018
@peterneyens peterneyens deleted the client-cache branch July 9, 2018 17:02
@peterneyens peterneyens changed the title Add client chache sub project Add client cache sub project Jul 9, 2018
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

Successfully merging this pull request may close these issues.

None yet

4 participants