-
Notifications
You must be signed in to change notification settings - Fork 18
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 missing implementation for casbin@0.3.0 and make unique key constraint works #15
Conversation
@GopherJ why don't use the built-in services provided by Travis CI? Like how we did it in Xorm adapter: https://github.com/casbin/xorm-adapter/blob/79a2aa54a016320eb29cf90090f642183827750b/.travis.yml#L14-L16 |
hello @hsluoyz we are using it, but in dev I would like to have some scripts so I added these two scripts |
Got it. |
.collect::<Vec<NewCasbinRule>>(); | ||
|
||
conn.transaction::<_, DieselError, _>(|| { | ||
diesel::insert_into(casbin_rules) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need the transaction here? I think Diesel will generate one query.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was also thinking about this. But I'm not really sure about it. I'm not sure if it's one query or multiple. If it's one query we can then remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also not 100% sure in every case 🙈
Hello, this PR make unique key works for policy rules, before it cannot work because we do have nullable fields.
Now we are adding empty string to solve this issue.
Besides, it adds implementation of
add_policies
andremove_policies
using diesel transaction.