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 Priority Queue library to sdk #6664

Merged
merged 17 commits into from
May 16, 2019
Merged

Add Priority Queue library to sdk #6664

merged 17 commits into from
May 16, 2019

Conversation

catsby
Copy link
Contributor

@catsby catsby commented May 1, 2019

This pull request adds a Priority Queue package to Vault's SDK. The first use-case will be used by plugins to maintain an in-memory priority queue to perform periodic actions on, which will be added in upcoming pull requests following this one.

@catsby catsby added this to the 1.2 milestone May 1, 2019
* master:
  Fixed Typo
  [Doc]: PKI Fix allowed_uri_sans spelling mistake (#6660)
  DynamoDB: Make Unlock key delete conditional on being old leader's (#6637)
  Fix hook by using env to discover the correct location of bash as sh doesn't have [[
sdk/queue/README.md Show resolved Hide resolved
sdk/queue/priority_queue.go Outdated Show resolved Hide resolved
sdk/queue/priority_queue.go Outdated Show resolved Hide resolved
sdk/queue/priority_queue.go Show resolved Hide resolved
sdk/queue/priority_queue.go Outdated Show resolved Hide resolved
sdk/queue/priority_queue.go Outdated Show resolved Hide resolved
sdk/queue/priority_queue.go Outdated Show resolved Hide resolved
sdk/queue/priority_queue.go Outdated Show resolved Hide resolved
sdk/queue/priority_queue.go Outdated Show resolved Hide resolved
sdk/queue/priority_queue.go Outdated Show resolved Hide resolved
sdk/queue/priority_queue.go Outdated Show resolved Hide resolved
sdk/queue/priority_queue.go Outdated Show resolved Hide resolved
sdk/queue/priority_queue.go Outdated Show resolved Hide resolved
Copy link
Member

@mgaffney mgaffney left a comment

Choose a reason for hiding this comment

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

Just one comment.

sdk/queue/priority_queue.go Outdated Show resolved Hide resolved
sdk/queue/priority_queue.go Outdated Show resolved Hide resolved
sdk/queue/priority_queue.go Outdated Show resolved Hide resolved
sdk/queue/priority_queue.go Outdated Show resolved Hide resolved
type PriorityQueue struct {
// data is the internal structure that holds the queue, and is operated on by
// heap functions
data queue
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be a pointer to the queue instead of the queue object? I see other places where you just pass the reference and that could just be stored here.

Copy link
Contributor Author

@catsby catsby May 8, 2019

Choose a reason for hiding this comment

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

So that I understand what you're asking, are the other places heap methods like heap.Init(&pq.data) and heap.Pop(&pq.data).(*Item), and you're asking if we make data a *queue instead of queue, so those calls are heap.Init(pq.data) et. al?

Assuming that's what you're asking, is there an advantage to doing that? I'm not opposed to it I'm just curious of the benefits.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine either way, it just seems that you are always using the reference to it in all calls instead of just storing the reference and just using it directly. It is really just a stylistic thing. If I see a bunch of & for the same thing, I start digging into the implementation.

@catsby catsby requested a review from briankassouf May 9, 2019 21:20
briankassouf
briankassouf previously approved these changes May 15, 2019
@catsby catsby merged commit 593c065 into master May 16, 2019
catsby added a commit that referenced this pull request May 16, 2019
* master: (85 commits)
  UI icon - add size (#6736)
  Add Priority Queue library to sdk (#6664)
  Add spellcheck="false" to form fields (#6744)
  Maximum typo in Vault UI (#6743)
  docs: Fix Markdown formatting error in AWS Auth (#6745)
  changelog++
  Update OIDC Provider Setup docs (#6739)
  Update to use newer sdk
  Copy LogInput from audit package, add OptMarshaler interface  (#6735)
  docs: fixed typo (#6732)
  Fix typo
  changelog++
  Use Go modules in CircleCI (#6729)
  Fix recovery key backup path documentation
  Vendoring updated grpc
  Add link to R client on libraries list (#6722)
  UI ember engines (#6718)
  changelog++
  Update grpc and protos (#6725)
  changelog++
  ...
@kalafut kalafut deleted the cdcr-priority-queue branch December 7, 2021 20:53
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.

5 participants