Skip to content
This repository has been archived by the owner on Sep 23, 2022. It is now read-only.

Fix the http-client performance problem #77

Merged
merged 4 commits into from
Mar 17, 2016
Merged

Conversation

danpersa
Copy link
Contributor

Tip: Don't block inside of actors 💯

authService: AuthService,
metrics: RouteMetrics)(
implicit
val authService: AuthService,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do it in one line: implicit val

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can't do that :( that's how scalariform formats it... I tried to change the style, but it seems it's not possible... 👎

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a sign, even scalariform doesn't like implicits :)

Copy link

Choose a reason for hiding this comment

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

Oh, interesting... perhaps worth filing a bug at https://github.com/scala-ide/scalariform ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is already a ticket scala-ide/scalariform#214

@c00ler
Copy link
Contributor

c00ler commented Mar 17, 2016

👍

@danpersa
Copy link
Contributor Author

Closes #70

danpersa added a commit that referenced this pull request Mar 17, 2016
Fix the http-client performance problem
@danpersa danpersa merged commit b8d204a into master Mar 17, 2016
authUserToRoute(team)
}
case ServiceResult.Failure(Ex(ex)) => {
logger.error(s"OAuthService failed with exception $ex")
Copy link

Choose a reason for hiding this comment

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

Just making sure that you've omitted including ex in the error call (i.e. error(ex, "") which includes the exception, not just message then)

@ktoso
Copy link

ktoso commented Mar 17, 2016

Looks good 👍
The logging nitpicks feel free to ignore, I see you explained why in one of the comments already

// Hope you don't mind me coming in to comment here, was at @danpersa's request :)

@danpersa danpersa deleted the fix-the-http-client branch May 23, 2016 16:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants