-
Notifications
You must be signed in to change notification settings - Fork 35
Add extra admin roles #183
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
Conversation
Signed-off-by: Mark Nelson <mark.x.nelson@oracle.com>
Signed-off-by: Mark Nelson <mark.x.nelson@oracle.com>
Signed-off-by: Mark Nelson <mark.x.nelson@oracle.com>
Signed-off-by: Mark Nelson <mark.x.nelson@oracle.com>
if strings.ToUpper(e.config.DbRole) == "SYSOPER" { | ||
P.IsSysOper = true | ||
switch strings.ToUpper(e.config.DbRole) { | ||
case "SYSDBA": |
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.
This is purely a style recommendation:
- Create a new type for this rather than using string literals
- switch on the new type
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'd also segregate database role handling code to it's own function/file. gonna suggest database_role.go
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.
updated to use the AdminRole type from godror, which handles parsing the string, deals with unknown values correctly, etc.
Signed-off-by: Mark Nelson <mark.x.nelson@oracle.com>
Allow the use to connect with additional admin roles.
Note that this uses a commit hash in godror, I plan to update this to a release when they do a release with the necessary change in it, and before we do a release with this commit.
Fixes #180