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

Split path into outerPath and innerPath, added innerRoute into server (W... #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benjaminjackman
Copy link
Contributor

...ill only look at inner paths inside requests)

Signed-off-by: Ben Jackman ben@jackman.biz

… (Will only look at inner paths inside requests)

Signed-off-by: Ben Jackman <ben@jackman.biz>
@benjaminjackman benjaminjackman mentioned this pull request Jan 8, 2015
@benjaminjackman
Copy link
Contributor Author

There is some previous discussion in #21

@benjaminjackman
Copy link
Contributor Author

I have been looking into doing similar stuff, I am going to be working on this today, so it's still a bit hypothetical
The pattern I am attempting is to have an ApiDirectory which has sub-api's within itself.

trait CgtaApiDirectory {
  val marketdataApi : MarketdataApi
  val orderdataApi : OrderdataApi
}

then I make my client, in the doCall method switch on the first element of the inner path.

CgtaApiClient(cfg : Servers) extends autowire.Client{
...
override def doCall(req : Request) {
    req.innerPath.toList match {
​​      case "marketdataApi" :: rest => httpRequest(cfg.mdServerUrl, req.copy(innerPath = rest).serialize)

   }
  }
}

then on the server side, I can make a server that handles just the marketdataApi with something like

MarketdataServer extends autowire.Server

onRequest { httpReq =>
  //Notice MarketdataApi, NOT CgtaApiDirectory
  val myMdServerImpl = new MdServerApiImpl()
  val router =  MarkdataServer.innerRouter[MarketdataApi](myMdServerImpl)
  router(httpReq.payload)
}

Having that string "marketdataApi" in the client sucks because if the variable get's renamed there is not compile time break so I was thinking about making a macro that converts a member name to a string (memberNamesOf[CgtaApiDirectory].marketdataApi) so if someone renames that member it will break at compile time.

Edit: couple typos

@lihaoyi
Copy link
Owner

lihaoyi commented Jan 9, 2015

Having thought about this more I think the @key approach would be better. Having two hard-coded identical macros is kind of gross, and there would never be a case where both macros were useful; you'd always only want one or the other, if I'm not mistaken? If we go with @key, then you won't have anything to choose from, and it'll always do the right thing.

For your use case, you can trivially set the key to empty, in which case you'll get the local path only, and it'll also be useful for people who want to use the default outer-path, but want to move the file while keeping the outer path the same (e.g. to not-break already-deployed clients).

WDYT?

@benjaminjackman
Copy link
Contributor Author

Traits that are defined before the autowire dependency is included cant
annotate with @key. The macros have a very slight difference maybe there is
a way to have one macro by having a default parameter (route only on inner
classes) or something. For my use cases so far the outer path or key has
had no real use, and i dont understand how it is needed or superior to
simply nesting apis inside a larger api trait. Maybe i am just not getting
it but id like to see a code example of how it is used because maybe i am
missing something in my design.

I think about it this way, without the outer path the macro just invokes
methods on any trait simply by using the string name + args. Its like
reflection.invokeMethodByNameSomeApi(path, args) for
this method adding a key or full classpath just i redundant.

Granted there might be a use case for the client side having the outer path
or key and passing it over the wire to the http server, or dispatching to
different urls based on its value (i still think mixins nesting is better
here) so at the top level of the http server you might want to switch on it
to choose which autowire.Server.route... to use.

Sent from my tablet.
On Jan 8, 2015 11:02 PM, "Li Haoyi" notifications@github.com wrote:

Having thought about this more I think the @key approach would be better.
Having two hard-coded identical macros is kind of gross, and there would
never be a case where both macros were useful; you'd always only want
one or the other, if I'm not mistaken? If we go with @key, then you won't
have anything to choose from, and it'll always do the right thing.

For your use case, you can trivially set the key to empty, in which case
you'll get the local path only, and it'll also be useful for people who
want to use the default outer-path, but want to move the file while keeping
the outer path the same (e.g. to not-break already-deployed clients).

WDYT?


Reply to this email directly or view it on GitHub
#22 (comment).

@lihaoyi
Copy link
Owner

lihaoyi commented Jan 10, 2015

One difference is that I don't want to have to nest two objects/methods awkwardly to rename something =P The @key lets you skip the outer-path if you want, and additionally provides a nice way of moving objects around into different packages in a backwards-compatible way.

My imagined use case is to simply chain up multiple routes with orElse since they're just partial functions. I don't manually switch on the objects much, but then again I use autowire for "heck it" routes where the path doesn't matter at all and certainly isn't outward facing. The fact that there's some "path" doesn't matter to me at all. I just want to be able to call methods on the server and get back futures.

From the way you're describing it, it seems that you may be trying to make autowire fit some pre-defined routing pattern that you already have implemented? That's probably not something that I imagined autowire to do. It's the Scala code that should matter and be kept as simple as possible.

@lihaoyi
Copy link
Owner

lihaoyi commented Jan 10, 2015

The idea is if you have

object Thing extends Api{
   ...
}

And you decide to rename it to something more sensible

object ViewControllerFactory extends Api{
   ...
}

You could do so without breaking anyone via a @key attribute

@key("thing")
object ViewControllerFactory extends Api{
   ...
}

Not sure if that makes sense. For one, it still doesn't allow you do to use the old & the new route simultaneously, so maybe it's not that useful.

@lihaoyi
Copy link
Owner

lihaoyi commented Jan 10, 2015

I'm still mulling over the consequences of routing to nested objects. What exactly do you use it for?

@benjaminjackman
Copy link
Contributor Author

I have several traits I want to be able to call from client side. I group all of those traits together into a larger 'API'. Nesting allows for that

For example (this is just illustrative):

trait TradingApi {
  val cmeMd : Marketdata
  val espeedMd : Marketdata
  val manualControls : TradingControls
  ...
}

trait Marketdata {
  def getSnapshot(symbol : Symbol, utc : UtcT)
}

trait TradingControls {
  def goWide(symbol : FirmSymbol, amount : Int)
  def haltTrading(symbol : FirmSymbol)
  ...
}

trait CompanyApi {
  val trading : TradingApi
  val reports : ReportingApi
  ...
}

I can then switch in the client implementation and send cmeMd requests to one server and espeed to another. The requests are sent over HTTP because it's easy to debug and understand, and if we need to do things like proxy or cache, we can use any off the shelf tool for that.

@lihaoyi
Copy link
Owner

lihaoyi commented Jan 13, 2015

Would something like this work?

@key("CompanyApi.TradingApi")
trait TradingApi {
  val cmeMd : Marketdata
  val espeedMd : Marketdata
  val manualControls : TradingControls
  ...
}
@key("CompanyApi.MarketData")
trait Marketdata {
  def getSnapshot(symbol : Symbol, utc : UtcT)
}
@key("CompanyApi.TradingControls")
trait TradingControls {
  def goWide(symbol : FirmSymbol, amount : Int)
  def haltTrading(symbol : FirmSymbol)
  ...
}

Not the exact syntax, but the idea of using @key to perform the nesting-of-paths, without needing to actually nest the actual things.

@benjaminjackman
Copy link
Contributor Author

cmeMd and espeedMd would collide in this example I think

@benjaminjackman
Copy link
Contributor Author

the other benefit to nesting is it really lowers the bar to what can be autowired, so even if you don't want to structure a nested api you can still use nesting where logical in existing traits. I just think if I am designing the API to be used only inside the server, I would use nesting, nothing as insane as the cake pattern mind you, but I would still nest the apis at some point.

With nesting, and having a mapping of nesting to requests, Autowire allows presenting the same API as what I would use internally, where I would not be using annotations.

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.

2 participants