-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
Create users if not found. #1753
Create users if not found. #1753
Conversation
username=user_data['username'], | ||
first_name=user_data['username'], | ||
last_name=user_data['username'], | ||
email=user_data['username'], |
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.
why are we using username
as email?
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.
typo :) will improve the unit test
'usernames': ['gamma'], | ||
'users': [{ | ||
'username': 'gamma', | ||
'email': '[email protected]' |
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 guessing this email won't actually get used to send email, but should we use the airbnb domain instead just in case it does and superset.com
has a catchall email address setup? looks like a parked domain currently.
could use [email protected]
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've added the email field, actually we plan to send emails for the access requests )
if not user and user_data['username']: | ||
sm.add_user( | ||
username=user_data['username'], | ||
first_name=user_data['username'], |
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.
does user_data
have a first/last name available?
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.
good point, I'll do it more generic, we do not have the data.
probably want @mistercrunch do take a quick look, but LGTM! |
LGTM |
This change will allow to pre-create and preassign users to the groups before they login to the superset. It will allow users to have full access once they are logged in to the tool.
Reviewers: