Skip to content

Add IdentityServer data migrator#9

Open
lukaszdobrzynski wants to merge 4 commits intoravendb:masterfrom
lukaszdobrzynski:master
Open

Add IdentityServer data migrator#9
lukaszdobrzynski wants to merge 4 commits intoravendb:masterfrom
lukaszdobrzynski:master

Conversation

@lukaszdobrzynski
Copy link
Collaborator

IdentityServerDataMigrator is an extra feature to make transition from in-memory configuration to RavenDB easier.
Also decided that it is a user that should be responsible for registering AfterDispose handler and its logic.

@lukaszdobrzynski lukaszdobrzynski force-pushed the master branch 2 times, most recently from 94cb747 to 28bf214 Compare August 20, 2021 16:54

if (documentStore.Certificate != null)
{
documentStore.AfterDispose += (sender, args) =>
Copy link
Member

Choose a reason for hiding this comment

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

Why remove the dispose here?

Copy link
Collaborator Author

@lukaszdobrzynski lukaszdobrzynski Aug 23, 2021

Choose a reason for hiding this comment

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

I believe it is a user that should be responsible for defining the handler's logic and be aware that a certificate needs to be displosed of along with the displosal of the document store. Do you think the library should perform the check and displose of the resource quitely?

Copy link
Member

Choose a reason for hiding this comment

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

The problem is that if this isn't disposed, there is a temp file that isn't being cleaned.
This is probably disposed because otherwise users tend to not pat attention to that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍 reverted

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