Skip to content

reaper: add support for a pre-delete hook#11

Open
yanniszark wants to merge 6 commits intoyosssi:masterfrom
arrikto:feature-reaper-predelete-fn
Open

reaper: add support for a pre-delete hook#11
yanniszark wants to merge 6 commits intoyosssi:masterfrom
arrikto:feature-reaper-predelete-fn

Conversation

@yanniszark
Copy link
Copy Markdown

@yanniszark yanniszark commented May 13, 2020

Resolves: #10

Changes organized in commits:

  • Add Go module support (go mod init)
  • Fix small formatting errors inside logging functions.
  • Add a PredeleteFn hook in reaper's options. Reaper will call the PredeleteFn with the session values. If the PredeleteFn returns an error, the key won't be deleted.
  • Fix unit tests:
    • Load unit test hanged, presumably because of a deadlock, because it opened a write transaction while inside a read transaction.
    • Check that keys are actually reaped for reaper unit test.
    • Protobuf error seems to have changed, so update it.

The wercker test doesn't pass because of a very old go version I presume.
Is there any way to update the box? Using docker images doesn't seem to work (I specified golang:1.14)

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>
@yanniszark yanniszark force-pushed the feature-reaper-predelete-fn branch from c8c392e to 60e395d Compare May 13, 2020 16:18
Add reaper option for a PreDeleteFn. This function runs before an
expired session is deleted. This provide the library user an extra hook
to do additional work before deleting a session (e.g., revoke tokens to
a remote endpoint).

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>
Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>
@yanniszark yanniszark force-pushed the feature-reaper-predelete-fn branch from 60e395d to 7096405 Compare May 13, 2020 19:34
@kjmrknsn
Copy link
Copy Markdown
Collaborator

Thanks for the PR. Is it ready for review?

The load unit-test hanged indefinitely. Presumably, this is because of a
deadlock caused by starting a write transaction inside a read
transaction. From the boltdb docs:

> Read-only transactions and read-write transactions should not depend on
> one another and generally shouldn't be opened simultaneously in the
> same goroutine. This can cause a deadlock as the read-write transaction
> needs to periodically re-map the data file but it cannot do so while a
> read-only transaction is open.

In addition, the protobuf error message returned for a case of the load
unit test seems to have changed, so amended to the latest text.

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>
@yanniszark
Copy link
Copy Markdown
Author

@kjmrknsn thanks for the prompt response!
Yes, the PR is ready for review.
I have updated the PR description in order to better reflect what changes were made.
Please let me know if something is unclear.

@yanniszark
Copy link
Copy Markdown
Author

@kjmrknsn kind ping on this! The CI tests are not passing because they need a newer go version, but it seems that a box is not available. Not very familiar with wercker. How can we have a box with a later golang version?

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.

reaper: add support for a pre-delete hook

2 participants