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

PXD-2730: client permission #53

Merged
merged 4 commits into from
May 1, 2019
Merged

PXD-2730: client permission #53

merged 4 commits into from
May 1, 2019

Conversation

fantix
Copy link
Contributor

@fantix fantix commented Apr 8, 2019

New Features

  • Added client
  • Check client permission

@fantix fantix requested a review from rudyardrichter April 8, 2019 20:33
arborist/auth.go Outdated
func _authorize(stmts *CachedStmts, exp Expression, args []string,
query string, qargs ...interface{}) (bool, error) {
// run authorization query
rows, err := stmts.Query(query, qargs...)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should be able to just do something like this:

var results []bool
err = db.Select(&results, query)

and be able to skip iterating through rows.Next()

Copy link
Contributor

Choose a reason for hiding this comment

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

(or I guess probably pq.BoolArray or something)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh here we actually needed a map of argName: value, so iteration is hardly avoidable I think. But I added the Select to the cached statements anyway ;)

@fantix fantix force-pushed the feat/client branch 2 times, most recently from d750a00 to 55545ce Compare April 24, 2019 23:13
@fantix fantix requested a review from rudyardrichter April 25, 2019 15:21
Copy link
Contributor

@rudyardrichter rudyardrichter left a comment

Choose a reason for hiding this comment

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

  • It looks like some files could use a go fmt run

@fantix
Copy link
Contributor Author

fantix commented May 1, 2019

@rudyardrichter I'm merging this to feat/db now, but I'm happy to make further changes if you found anything else ;) thanks a lot!

@fantix fantix merged commit dddbaf4 into feat/db May 1, 2019
@fantix fantix deleted the feat/client branch May 1, 2019 18:29
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.

3 participants