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

ClientConfiguration vs OrleansConfiguration assymetry #41

Closed
yevhen opened this issue Jan 28, 2015 · 6 comments
Closed

ClientConfiguration vs OrleansConfiguration assymetry #41

yevhen opened this issue Jan 28, 2015 · 6 comments
Assignees

Comments

@yevhen
Copy link
Contributor

yevhen commented Jan 28, 2015

While I can perfectly load OrleansConfiguration using

new OrleansConfiguration().Load(new StringReader(embeddedResourceFile));

The ClientConfiguration does not provide this method in it's public interface, so currently I'm using this filthy hack:

var loader = result.GetType().GetMethod("Load", BindingFlags.Instance | BindingFlags.NonPublic, null, new[] {typeof(TextReader)}, null);
loader.Invoke(result, new object[]{LoadFromEmbeddedResource(assembly, fullResourcePath)});

Also, I think renaming OrleansConfiguration to 'ServerConfiguration` will add consistency and perfect symmetry (Client/Server). The principle of "no surprises" - is a hallmark of a good API =))

@gabikliot
Copy link
Contributor

There is no more OrleansConfiguration. We renamed the class to ClusterConfiguration. The file is still called OrleansConfiguration.cs and we should change that.

Yes in favor of adding ClientConfiguration.Load(new StringReader(embeddedResourceFile));

@yevhen
Copy link
Contributor Author

yevhen commented Jan 28, 2015

Great!

@jthelin
Copy link
Member

jthelin commented Jan 29, 2015

I renamed the files to avoid confusion. PR #45

Adding symmetrical ClientConfiguration.Load method should be done as separate PR.

@jthelin
Copy link
Member

jthelin commented Jan 29, 2015

I created PR #47 to change ClientConfiguration.Load method public.

@gabikliot - could you take a look at this PR because it seems like the AdjustConfiguration function got disconnected somewhere along the way, but isn't it needed for some streaming providers?

@jthelin jthelin self-assigned this Jan 29, 2015
@gabikliot
Copy link
Contributor

I will.

@gabikliot
Copy link
Contributor

Merged #47.

sebastianburckhardt pushed a commit to sebastianburckhardt/orleans that referenced this issue Apr 24, 2017
Add extension methods for configuring transaction log storage.
Transaction log storage must be configured now, does not default to memory storage anymore.
Transaction log storage automatically configured in Azure Client and Silo default configurations.
Add memory transaction log configuration for client config.
@ghost ghost locked as resolved and limited conversation to collaborators Sep 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants