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

Fix client side EHStreamProvider init #3096

Merged
merged 1 commit into from
Jun 14, 2017

Conversation

xiazen
Copy link
Contributor

@xiazen xiazen commented Jun 6, 2017

GlobalConfig and NodeConfig is not available in client side. So those two shouldn't be required.

@jason-bragg
Copy link
Contributor

Was this causing any issues? Create cache factory should never be called on the client, so I'd think the current code should be fine? I ask because when these types are needed, they -are- required..?

@xiazen
Copy link
Contributor Author

xiazen commented Jun 6, 2017

yes the function CreateCacheFactory itself is called when init, in client and silo side. Remember we wrap all the Cache related customization into EventHubQueueCacheFactory, and CreateCacheFactory is called to create the EventHubQueueCacheFactory

@jason-bragg
Copy link
Contributor

Ah, I see.
My preference would be to not initialize things on the client that are not needed their, rather than run the logic without the necessary data.
I don't see an easy way of doing that off the top of my head, but some effort on this now will likely save us some bugs down the line.

@xiazen
Copy link
Contributor Author

xiazen commented Jun 8, 2017

I will look into it.

@xiazen xiazen added this to the 1.5.0 milestone Jun 8, 2017
@jdom
Copy link
Member

jdom commented Jun 14, 2017

Ping. Is there any progress with this? Since this is currently broken. I'd prefer to just merge this PR if no progress will be made very soon

@jason-bragg jason-bragg merged commit 0543f48 into dotnet:master Jun 14, 2017
@xiazen xiazen deleted the fix-client-side-statistics branch November 1, 2017 20:32
@github-actions github-actions bot locked and limited conversation to collaborators Dec 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants