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 a "leasedby" column #16

Merged
merged 4 commits into from
Sep 1, 2017
Merged

Add a "leasedby" column #16

merged 4 commits into from
Sep 1, 2017

Conversation

MrMDavidson
Copy link
Contributor

For now just a column useful for introspection scenarios. In future it may be used to further validate the lease (eg. Option to not allow renewal / forceful takeover of a lease if it belongs to someone else. Needs thought for cloud scenarios where entire workers may vanish)


Rebus is MIT-licensed. The code submitted in this pull request needs to carry the MIT license too. By leaving this text in, I hereby acknowledge that the code submitted in the pull request has the MIT license and can be merged with the Rebus codebase.

For now just a column useful for introspection scenarios. In future it may be used to further validate the lease (eg. Option to not allow renewal / forceful takeover of a lease if it belongs to someone else. Needs thought for cloud scenarios where entire workers may vanish)
@@ -210,7 +210,7 @@ static void InitializeDatabase(string databaseName)
static string GetConnectionStringForDatabase(string databaseName)
{
return Environment.GetEnvironmentVariable("REBUS_SQLSERVER")
?? $"server=.; database={databaseName}; trusted_connection=true;";
?? $"server=.\\SqlExpress; database={databaseName}; trusted_connection=true;";
Copy link
Member

Choose a reason for hiding this comment

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

???

😁

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

@MrMDavidson
Copy link
Contributor Author

MrMDavidson commented Aug 28, 2017 via email

@mookid8000
Copy link
Member

😁

@MrMDavidson
Copy link
Contributor Author

I don't think I can re-run the build for this... but the it's failing because of a file lock it seems. Re-running it should fix it.

@mookid8000 mookid8000 merged commit 95c2d10 into rebus-org:master Sep 1, 2017
@mookid8000
Copy link
Member

excellent work! 😁

it will take a little while for me to go through this .... it'll be out in Rebus.SqlServer 5 pre, probably a little later today

@MrMDavidson
Copy link
Contributor Author

Awesome :) Will be nice to switch to using a nuget package instead of my copy/paste versions of SqlTransport and SqlLeaseTransport into our project. The lease transport is working really well for us and as a bonus it means inspection of the queue is really easy as the rows are virtually never locked and the leasedby/until columns provide great debugging and visibility.

@mookid8000
Copy link
Member

super!

one thing, though – is it intentional that there is no one-way client configuration for the lease-based transport?

@MrMDavidson
Copy link
Contributor Author

Not intentional, no, just not something I'm familiar with. Is that an easier set up path where people only insert into the queue but don't process it? Can't see any technical reason that wouldn't work.

@mookid8000
Copy link
Member

Yes, it's simply a send-only Rebus endpoint, i.e. one that doesn't have an input queue, avoiding the need to spin up worker threads, etc.

I'll go add it – as the transport seems to satisfy the contract tests, e.g. accepting to be instantiated with a null input queue address, I think it is only about two missing configuration exetnsions....

@MrMDavidson
Copy link
Contributor Author

Yeah - if SqlTransport supports it then SqlLeaseTransport should as it just changes the way messages are de-queued and rollbacks are performed.

@mookid8000
Copy link
Member

it's out in 5.0.0-b1 now 😄

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.

2 participants