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

F# API changes #526

Merged
merged 6 commits into from
Dec 13, 2014
Merged

F# API changes #526

merged 6 commits into from
Dec 13, 2014

Conversation

Horusiath
Copy link
Contributor

New features

  1. Included comments for all public functions.
  2. All functions now have explicit signatures - this way we won't allow for accidental API breaking changes.
  3. Created logging functions:
    • log (level : LogLevel) (mailbox : Actor<'a>) (msg: string) : unit
    • logDebug (mailbox : Actor<'a>) (msg: string) : unit
    • logInfo (mailbox : Actor<'a>) (msg: string) : unit
    • logWarning (mailbox : Actor<'a>) (msg: string) : unit
    • logError (mailbox : Actor<'a>) (msg: string) : unit
    • logException (mailbox : Actor<'a>) (e: exn) : unit
  4. Created equivalent of C# PipeTo method working on F# Async<'a> computations - I'm not 100% sure if this will work correctly:
    • pipeTo (computation : Async<'t>) (recipient : ICanTell) (sender : ActorRef) : unit
    • (|!>) (computation : Async<'t>) (recipient : ICanTell) - pipe to equivalent used for forward async computation result to right-side recipient with NoSender
    • (<!|) (recipient : ICanTell) (computation : Async<'t>) - pipe to equivalent used for forward async computation result to left-side recipient with NoSender
  5. Inbox functional API:
    • inbox (system : ActorSystem) : Inbox - equivalent of Inbox.Create
    • receive (timeout : TimeSpan) (i : Inbox) : 'm option - equivalent of Inbox.Receive but statically typed. Returns option with None result on timeout exceeded or result of incompatible type returned.
    • filterReceive (timeout : TimeSpan) (predicate : 'm -> bool) (i : Inbox) : 'm option - equivalent of Inbox.ReceiveWhere, again statically typed. Returns option with None result on timeout exceeded or result of incompatible type returned.
    • asyncReceive (i : Inbox) : Async<'m option> - equivalent of Inbox.ReceiveAsync. Async computation should return None result on result of incompatible type returned. This may not be a correct signature and return type may change

Breaking changes

  1. There is no separate spawning method for actors now. Actor system and actor implementations may both use the same spawning functions now.
  2. Removed spawns function. Instead spawnp function was created spawnp (system : ActorRefFactory) (name : string) (mapParams : PropsParams -> PropsParams) (f : Actor<'m> -> Cont<'m, 'v>) : ActorRef. Map params is a mapping function used for specifying all additional parameters to be applied onto inner Props object used for actor creation. This is more comprehensive implementation than spawns, which could be used only for defining a SupervisorStrategy.
  3. spawne also takes additional (mapParams : PropsParams -> PropsParams) parameter.
  4. select function has reversed input parameters. See F# API open propositions #513 for motivation.

Examples updated + one new example

All of the changes above have been applied to F# API examples. Additionally there is a new sample showing use of F# spawning function for Remote Deployment feature - unlike the C# one, this is share nothing example, which means that there are no libraries with shared code. Actor logic is serialized as F# expression, send over the wire and then compiled and executed onto remote actor system!

@rogeralsing
Copy link
Contributor

Looks good as far as I can tell, but my F# skills is not enough to review this.
anyone else @akkadotnet/developers

@HCanber
Copy link
Contributor

HCanber commented Dec 1, 2014

I can't really review this either. I have one remark:

let inline private loggerFor (context : IActorContext) : LoggingAdapter = Logging.GetLogger(context)

let log (level : LogLevel) (mailbox : Actor<'a>) (msg: string) : unit = 
    let logger = loggerFor mailbox.Context
    logger.Log(level, msg)

let inline logDebug mailbox msg = log (LogLevel.DebugLevel) mailbox msg

Everytime one logs, loggerFor is called. Correct?
It will call GetLogger which really should be called CreateLogger as it creates a new BusLogging and a DefaultLogMessageFormatter. So those will be created on every log-call.

@Horusiath
Copy link
Contributor Author

Thx @HCanber . Indeed GetLogger name is misleading. I'll try to solve this problem differently - possibly lazy Logger field inside F# Actor? This way we avoid BusLogging and DefaultLogMessageFormatter creation if unnecessary, while still being able to use logging ability.

@HCanber
Copy link
Contributor

HCanber commented Dec 1, 2014

Yeah, that's what we recommended in C#.

public class MyActor : ReceiveActor
{
  private readonly LoggingAdapter _log = Context.GetLogger();
  ...
}

@Horusiath
Copy link
Contributor Author

Lazy logger creation pattern has been applied.

@Horusiath
Copy link
Contributor Author

I've renamed PropsParams config record to SpawnParams - initially this record was expected only to configure Props instance which is not accessible outside of the spawn functions, but I thought that it could be used in more fields in the future.

Additionally I've extended share-nothing remote actor deployment example. Now it illustrated a way of dealing with passing more complex data structures - since F# expression cannot serialize definitions of any types and functions declared outside of the expression itself. Closest-to-erlang solution is to passing message in form of a tagged tuple, but since there is no erlang atom equivalent in F#, I've used integer constant literals, which could be used for tagging instead.

PS: Waiting for positive contribution review ;)

[<Interface>]
type Actor<'msg> =
inherit ActorRefFactory
(** Explicitly retrieves next incoming message from the mailbox. *)
Copy link

Choose a reason for hiding this comment

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

Please document using /// XML-doc comments - which are standard for F# coding. (Unless there's some reason not to)

… arguments for spawning parameters - list of options
@Horusiath
Copy link
Contributor Author

Almost all of @dsyme comments has been taken into account. I want to discuss about two of them:

  1. Presence of <? and <! - as I've suggested before, it's justified since framework has strong associations with it's JVM and Erlang predecessors.
  2. I've looked into FSharp.Charting API and it reminded me about other solution of passing parameters (seen in handling data series collections), also found inside Erlang spawn_opt function. I've renamed function spawnp to spawnOpt (fac: ActorRefFactory) (name : string) (f : Actor<'T> -> Cont<'T, 'U>) (options : SpawnOption list) to reflect that behavior. Now it takes a list of SpawnOption (union type) objects. I think it's nicely corresponds to it's Erlang-inspired API, while still being an acceptable F# pattern.

@Aaronontheweb
Copy link
Member

@Horusiath I'm not an experienced F# developer, but these conventions seem reasonable to me.

@dsyme
Copy link

dsyme commented Dec 12, 2014

I'm OK with this - if I get the chance to start using the API (I hope to) then I may have more input. Please also get on twitter and encourage people to trial the API and give feedback. Some F# Interactive actor scripting samples would also be useful in that regard, for tutorial purposes.

@dsyme
Copy link

dsyme commented Dec 12, 2014

Will this bring the API docs up-to-date too? http://akkadotnet.github.io/wiki/FSharp%20API

@Horusiath
Copy link
Contributor Author

@dsyme API documentation needs a separate commit. I'll be more than happy to give some better insights on official Akka.NET site.

Current samples are available on this git repo under /src/examples directory. This commit also gives a new example of remote actor deployment feature for F# API and updates those, which have existed already.

@Horusiath
Copy link
Contributor Author

Can I ask, what is the state of this commit? Accordingly to F# Advent Calendar my tomorrow (Sunday) presentation should be about Akka.NET remote deployment with F#. I just like to know if I could use new API features for this purpose, or stay to the old one, or show both ;) (actually if we could deploy them on -Pre NuGet branch it would be awesome)

rogeralsing added a commit that referenced this pull request Dec 13, 2014
@rogeralsing rogeralsing merged commit 77321e5 into akkadotnet:dev Dec 13, 2014
Aaronontheweb added a commit to Aaronontheweb/akka.net that referenced this pull request Dec 13, 2014
__Brand New F# API__. The entire F# API has been updated to give it a
more native F# feel while still holding true to the Erlang / Scala
conventions used in actor systems. [Read more about the F# API
changes](akkadotnet#526).

__Multi-Node TestKit (Alpha)__. Not available yet as a NuGet package,
but the first pass at the Akka.Remote.TestKit is now available from
source, which allow you to test your actor systems running on multiple
machines or processes.

A multi-node test looks like this

public class InitialHeartbeatMultiNode1 : InitialHeartbeatSpec
{
}

public class InitialHeartbeatMultiNode2 : InitialHeartbeatSpec
{
}

public class InitialHeartbeatMultiNode3 : InitialHeartbeatSpec
{
}

public abstract class InitialHeartbeatSpec : MultiNodeClusterSpec
The MultiNodeTestRunner looks at this, works out that it needs to create
3 processes to run 3 nodes for the test.
It executes NodeTestRunner in each process to do this passing parameters
on the command line. [Read more about the multi-node testkit
here](akkadotnet#497).

__Breaking Change to the internal api: The `Next` property on
`IAtomicCounter<T>` has been changed into the function `Next()`__ This
was done as it had side effects, i.e. the value was increased when the
getter was called. This makes it very hard to debug as the debugger kept
calling the property and causing the value to be increased.

__Akka.Serilog__ `SerilogLogMessageFormatter` has been moved to the
namespace `Akka.Logger.Serilog` (it used to be in
`Akka.Serilog.Event.Serilog`).
Update your `using` statements from `using Akka.Serilog.Event.Serilog;`
to `using Akka.Logger.Serilog;`.

__Breaking Change to the internal api: Changed signatures in the
abstract class `SupervisorStrategy`__. The following methods has new
signatures: `HandleFailure`, `ProcessFailure`. If you've inherited from
`SupervisorStrategy`, `OneForOneStrategy` or `AllForOneStrategy` and
overriden the aforementioned methods you need to update their
signatures.

__TestProbe can be implicitly casted to ActorRef__. New feature. Tests
requring the `ActorRef` of a `TestProbe` can now be simplified:
``` C#
var probe = CreateTestProbe();
var sut = ActorOf<GreeterActor>();
sut.Tell("Akka", probe); // previously probe.Ref was required
probe.ExpectMsg("Hi Akka!");
```

__Bugfix for ConsistentHashableEvenlope__. When using
`ConsistentHashableEvenlope` in conjunction with
`ConsistentHashRouter`s, `ConsistentHashableEvenlope` now correctly
extracts its inner message instead of sending the entire
`ConsistentHashableEvenlope` directly to the intended routee.

__Akka.Cluster group routers now work as expected__. New update of
Akka.Cluster - group routers now work as expected on cluster
deployments. Still working on pool routers. [Read more about
Akka.Cluster routers
here](akkadotnet#489).
This was referenced Dec 13, 2014
@Aaronontheweb
Copy link
Member

@Horusiath should be live on NuGet now :)

@Horusiath
Copy link
Contributor Author

@Aaronontheweb That's great! Thank you all for help.

@Horusiath Horusiath deleted the fsapi-impl branch March 23, 2016 14:07
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

5 participants