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

Enh/apache superset #1425

Merged
merged 3 commits into from
Oct 17, 2022
Merged

Conversation

sheralim012
Copy link
Contributor

@sheralim012 sheralim012 commented Oct 17, 2022

#1420

  • superset admin users will create dataset and give ownership to current user

DevOps changes

  • add the following env. variables
SUPERSET_USERNAME
SUPERSET_PASSWORD

Copy link
Contributor

@istride istride left a comment

Choose a reason for hiding this comment

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

I think that the app should have a service account in Superset - an account that represents the app in Superset. This account would give the app permission to do whatever it needs to do to create dashboards. If this is what is intended with this PR then I think the settings should be called:

  • SUPERSET_USERNAME
  • SUPERSET_PASSWORD

I would also encourage access to dashboards (and datasets, etc) be given to groups (in Superset) rather than individual users. The groups can be managed within Superset to control the level of access.

On the Wagtail side, I'm not keen on requiring Wagtail admins to authenticate themselves with their Superset credentials to create dashboards in Superset. I think the ability to create dashboards from Wagtail should belong to a Wagtail group, to which, users will be added if they need to create dashboards.

@sheralim012 sheralim012 requested a review from istride October 17, 2022 16:11
@sheralim012
Copy link
Contributor Author

I think that the app should have a service account in Superset - an account that represents the app in Superset. This account would give the app permission to do whatever it needs to do to create dashboards. If this is what is intended with this PR then I think the settings should be called:

  • SUPERSET_USERNAME
  • SUPERSET_PASSWORD

you are right, updated the variables' names.

I would also encourage access to dashboards (and datasets, etc) be given to groups (in Superset) rather than individual users. The groups can be managed within Superset to control the level of access.

superset has roles/groups that have access/permissions and users will be added to those roles.

On the Wagtail side, I'm not keen on requiring Wagtail admins to authenticate themselves with their Superset credentials to create dashboards in Superset. I think the ability to create dashboards from Wagtail should belong to a Wagtail group, to which, users will be added if they need to create dashboards.

if we don't authenticate individual users then we can't create dashboards, charts, etc under those users.

@istride
Copy link
Contributor

istride commented Oct 17, 2022

if we don't authenticate individual users then we can't create dashboards, charts, etc under those users.

I'm suggesting that we only create dashboards under the app's service account, but give access to other Superset users via groups. There would be no ownership change, so no need to ask for credentials.

Copy link
Contributor

@istride istride left a comment

Choose a reason for hiding this comment

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

Feel free to merge if no other changes are required.

@sheralim012 sheralim012 merged commit da334fc into unicef:develop Oct 17, 2022
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.

2 participants