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

Add OriginatingHostId and ProcessingHostId headers that represent the current "host" #1905

Closed
johnsimons opened this issue Jan 16, 2014 · 9 comments
Assignees
Milestone

Comments

@johnsimons
Copy link
Member

We currently have the ProcessingMachine and OriginatingMachine headers. These do not accurately reflect all concepts of a "Host". For example when hosting in Azure.

So the headers OriginatingHostId, HostId and HostDisplayName have been added to be environment agnostic where "machine" does not make sense.

The headers ProcessingMachine and OriginatingMachine have been marked as obsolete and will be removed in 5.0.

@johnsimons
Copy link
Member Author

@andreasohlund @yvesgoeleven does this cover what we discussed?

@dannycohen
Copy link

@andreasohlund - this does not seem to allign with what we discussed in Particular/ServiceControl#180

also - changing the headers wil afect everything in SI in non backwards compatible way. Has this been considered ?

@johnsimons
Copy link
Member Author

@dannycohen that is the reason why is assigned to v4.4

@andreasohlund
Copy link
Member

We need to keep Headers.ProcessingEndpoint since that is the endpoint name which is still valid in azure.

Regarding machine: We need to do this in a backwards compatible way (goes with out saying) so the header will stay for one more major version

@johnsimons
Copy link
Member Author

But shouldn't the Headers.ProcessingEndpoint contains the endpoint instance
?

On Thursday, 16 January 2014, Andreas Öhlund notifications@github.com
wrote:

We need to keep Headers.ProcessingEndpoint since that is the endpoint name
which is still valid in azure.

Regarding machine: We need to do this in a backwards compatible way (goes
with out saying) so the header will stay for one more major version


Reply to this email directly or view it on GitHubhttps://github.com//issues/1905#issuecomment-32453071
.

@andreasohlund
Copy link
Member

Well, we need to know the endpoint that it belongs to as well. So either 2
headers or 1 header with a syntax that reveals both?

On Thu, Jan 16, 2014 at 10:34 AM, John Simons notifications@git.luolix.topwrote:

But shouldn't the Headers.ProcessingEndpoint contains the endpoint
instance
?

On Thursday, 16 January 2014, Andreas Öhlund notifications@github.com
wrote:

We need to keep Headers.ProcessingEndpoint since that is the endpoint
name
which is still valid in azure.

Regarding machine: We need to do this in a backwards compatible way
(goes
with out saying) so the header will stay for one more major version


Reply to this email directly or view it on GitHub<
https://github.com/Particular/NServiceBus/issues/1905#issuecomment-32453071>

.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1905#issuecomment-32453753
.

@johnsimons
Copy link
Member Author

We can do that since the endpoint instance includes the name

On Thursday, 16 January 2014, Andreas Öhlund notifications@github.com
wrote:

Well, we need to know the endpoint that it belongs to as well. So either 2
headers or 1 header with a syntax that reveals both?

On Thu, Jan 16, 2014 at 10:34 AM, John Simons <notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>>wrote:

But shouldn't the Headers.ProcessingEndpoint contains the endpoint
instance
?

On Thursday, 16 January 2014, Andreas Öhlund <notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>>

wrote:

We need to keep Headers.ProcessingEndpoint since that is the endpoint
name
which is still valid in azure.

Regarding machine: We need to do this in a backwards compatible way
(goes
with out saying) so the header will stay for one more major version


Reply to this email directly or view it on GitHub<

https://github.com/Particular/NServiceBus/issues/1905#issuecomment-32453071>

.


Reply to this email directly or view it on GitHub<
https://github.com/Particular/NServiceBus/issues/1905#issuecomment-32453753>

.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1905#issuecomment-32455025
.

@andreasohlund
Copy link
Member

endpoint instance includes the name

Can we really assume that? Especially if we allow users to override?

On Thu, Jan 16, 2014 at 11:05 AM, John Simons notifications@git.luolix.topwrote:

We can do that since the endpoint instance includes the name

On Thursday, 16 January 2014, Andreas Öhlund notifications@github.com
wrote:

Well, we need to know the endpoint that it belongs to as well. So either
2
headers or 1 header with a syntax that reveals both?

On Thu, Jan 16, 2014 at 10:34 AM, John Simons <notifications@github.com<javascript:_e({},
'cvml', 'notifications@github.com');>>wrote:

But shouldn't the Headers.ProcessingEndpoint contains the endpoint
instance
?

On Thursday, 16 January 2014, Andreas Öhlund <notifications@github.com<javascript:_e({},
'cvml', 'notifications@github.com');>>

wrote:

We need to keep Headers.ProcessingEndpoint since that is the
endpoint
name
which is still valid in azure.

Regarding machine: We need to do this in a backwards compatible way
(goes
with out saying) so the header will stay for one more major version


Reply to this email directly or view it on GitHub<

https://github.com/Particular/NServiceBus/issues/1905#issuecomment-32453071>

.


Reply to this email directly or view it on GitHub<

https://github.com/Particular/NServiceBus/issues/1905#issuecomment-32453753>

.


Reply to this email directly or view it on GitHub<
https://github.com/Particular/NServiceBus/issues/1905#issuecomment-32455025>

.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1905#issuecomment-32455856
.

@johnsimons
Copy link
Member Author

Implemented as part of #1913

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

No branches or pull requests

4 participants