Change Actor methods involving Reminders and Timers from protected to public

[ScottArbeit]

Describe the proposal

I propose that we change the declaration of methods involving Actor Reminders and Timers from protected to public.

The problem

I'm using Dapr Actors from F#, and I'm running into a problem with methods like RegisterReminderAsync() being protected instead of public.

Because they're protected, when I try to do something natural in F# like:

member private this.SchedulePhysicalDeletion(deleteReason, delay, correlationId) =
    task {
        try
            let tuple = (referenceDto.BranchId, referenceDto.DirectoryId, referenceDto.Sha256Hash, deleteReason, correlationId)
            let! _ = this.RegisterReminderAsync(ReminderType.PhysicalDeletion, toByteArray tuple, delay, TimeSpan.FromMilliseconds(-1))
            ()
        with ex ->
            log.LogError(ex, "Error scheduling physical deletion reminder.")
    }
    :> Task

I get error FS0491:

Severity	Code	Description
Error	FS0491	The member or object constructor 'RegisterReminderAsync' is not accessible. Private members may only be accessed from within the declaring type. [this part -->] Protected members may only be accessed from an extending type and cannot be accessed from inner lambda expressions.

I expect this has something to do with the way the Task Computation Expression rewrites the inner lambda. Anyway, the only way for me to call RegisterReminderAsync() is to block and wait for the call to finish:

member private this.SchedulePhysicalDeletionSynchronous(deleteReason, delay, correlationId) =
    let tuple = (referenceDto.BranchId, referenceDto.DirectoryId, referenceDto.Sha256Hash, deleteReason, correlationId)

    this
        .RegisterReminderAsync(ReminderType.PhysicalDeletion, toByteArray tuple, delay, TimeSpan.FromMilliseconds(-1))
        .Result
    |> ignore

The proposed solution

If I recompile Dapr with RegisterReminderAsync() changed from protected to public, the error goes away.

Because it was so simple to do, I've created a branch with the proposed changes that I've tested against.

Would a PR be welcomed for this?

Repercussions / breaking changes

None that I can think of. Existing code written against the SDK will work unchanged, and it will unblock me in F#.

[philliphoff]

@ScottArbeit My initial reaction is reluctance to make those methods public given their original intent to be used solely by the actor instance itself, especially if it's primarily needed as workaround for a limitation of one specific language (as much as I'd like to make SDK use from F# as fluid and idiomatic as C# itself).

Is another option to create a one-off public method on this particular actor type that just delegates to, for example, RegisterReminderAsync()? Is it really necessary to make the method(s) public for every actor instance?

[ScottArbeit]

@philliphoff Thank you for the idea, I'll give it a try and let you know. It'll take a couple of days, I'm traveling, but I'll respond this week.

For what it's worth, I find protected in general adds no value; I understand private, and I understand (but don't like) sealed as there are perf improvements you can get with it, but I never use protected and would always choose public, which is part of why I'm comfortable asking to update the API. I'm not sure what bad behavior is prevented by making these protected.

[ScottArbeit]

I finally got back to this (sorry about that). I have a simple workaround, I just needed to unblock my mind.

I'm still a little <grumpy-old-man-shaking-fist-at-clouds> about the protected stuff, but closing this,