[GitHub] [airflow] ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details
ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details URL: https://github.com/apache/airflow/pull/4569#issuecomment-475570674 Yes that probably makes sense to do. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details
ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details URL: https://github.com/apache/airflow/pull/4569#issuecomment-474986844 @feng-tao I haven't managed to finish it but I got somewhere with migrations to deal with perms: https://gist.github.com/ashb/f43741740fb0eae59948d52634cda575 The biggest issue is working out 1) what query to run, or 2) working out how to call the right classes/methods from within fab.security.* to update. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details
ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details URL: https://github.com/apache/airflow/pull/4569#issuecomment-469896394 Oh thanks, I hadn't seen that diagram! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details
ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag
details
URL: https://github.com/apache/airflow/pull/4569#issuecomment-469861187
I'm sort of picturing something like (very pseudo code):
```python
from something import role_migrations as rm
def up():
p = rm.add_permission('all_dags', 'can_dag_edit')
rm.add_to_role('Op', p)
def down():
rm.remove_from_role('Op', 'all_dags', 'can_dag_edit')
```
I haven't worked out the details yet, still a bit hazy (and I'm not 100% on
the FAB data model around permissions, view menus, roles and all the
many-to-many tables between them.)
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details
ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details URL: https://github.com/apache/airflow/pull/4569#issuecomment-469856650 Correct, yeah I've been testing the v1-10-stable branch to prepare 1.10.3 - though the problem would also apply to any upgraded install from 1.10 to what becomes master. So I feel we need to fix this in some way that gives an upgrade path form 1.10.x now to 2.0 as it will be - that doesn't involve manually adding things in the UI. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details
ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag
details
URL: https://github.com/apache/airflow/pull/4569#issuecomment-469828759
1. It never gets to L1657, `@has_dag_access` issues a 302 before the view fn
is ever run.
2. I'll cherry-pick this PR (which adds the change you suggest) in to 1.10.3
which will fix the problem for new installs. Adding all_dags to the existing
roles (I assume you mean Admin,User,Op, not all roles in the DB?) would "work"
but it seems counter to the purpose of #4118, of letting those roles be
editable by the admin of the site.
Let me have a look at how we might do this with a migration. If we could
(easily, without loads of code) replace all the role syncing code at run time,
which would also have the benefit of removing the mildly annoying "duplicate"
log lines:
```
[2019-03-05 19:38:25,622] {security.py:437} INFO - Start syncing user roles.
[2019-03-05 19:38:25,634] {security.py:437} INFO - Start syncing user roles.
[2019-03-05 19:38:25,677] {security.py:437} INFO - Start syncing user roles.
[2019-03-05 19:38:25,684] {security.py:437} INFO - Start syncing user roles.
[2019-03-05 19:38:25,988] {security.py:195} INFO - Existing permissions for
the role:Viewer within the database will persist.
[2019-03-05 19:38:26,085] {security.py:195} INFO - Existing permissions for
the role:Viewer within the database will persist.
[2019-03-05 19:38:26,172] {security.py:195} INFO - Existing permissions for
the role:Viewer within the database will persist.
[2019-03-05 19:38:26,183] {security.py:195} INFO - Existing permissions for
the role:Viewer within the database will persist.
[2019-03-05 19:38:26,520] {security.py:195} INFO - Existing permissions for
the role:User within the database will persist.
[2019-03-05 19:38:26,538] {security.py:195} INFO - Existing permissions for
the role:User within the database will persist.
[2019-03-05 19:38:26,692] {security.py:195} INFO - Existing permissions for
the role:User within the database will persist.
[2019-03-05 19:38:26,784] {security.py:195} INFO - Existing permissions for
the role:User within the database will persist.
[2019-03-05 19:38:27,158] {security.py:195} INFO - Existing permissions for
the role:Op within the database will persist.
[2019-03-05 19:38:27,165] {security.py:195} INFO - Existing permissions for
the role:Op within the database will persist.
[2019-03-05 19:38:27,166] {security.py:195} INFO - Existing permissions for
the role:Op within the database will persist.
[2019-03-05 19:38:27,169] {security.py:346} INFO - Fetching a set of all
permission, view_menu from FAB meta-table
[2019-03-05 19:38:27,170] {security.py:346} INFO - Fetching a set of all
permission, view_menu from FAB meta-table
[2019-03-05 19:38:27,173] {security.py:195} INFO - Existing permissions for
the role:Op within the database will persist.
[2019-03-05 19:38:27,177] {security.py:346} INFO - Fetching a set of all
permission, view_menu from FAB meta-table
[2019-03-05 19:38:27,203] {security.py:346} INFO - Fetching a set of all
permission, view_menu from FAB meta-table
[2019-03-05 19:38:28,254] {security.py:299} INFO - Cleaning faulty perms
[2019-03-05 19:38:28,266] {security.py:299} INFO - Cleaning faulty perms
[2019-03-05 19:38:28,268] {security.py:299} INFO - Cleaning faulty perms
[2019-03-05 19:38:28,289] {security.py:299} INFO - Cleaning faulty perms
```
(Cos each worker runs the checks, we do this for every worker at start, and
then every 30s as the new worker cycles).
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details
ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details URL: https://github.com/apache/airflow/pull/4569#issuecomment-469798406 A possibly simpler use case: Start with a fresh install on 1.10.2, add a user in the "Op" role and try to pause a DAG :) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details
ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details URL: https://github.com/apache/airflow/pull/4569#issuecomment-469796095 @feng-tao Okay I've got a reproduction case, I'm not sure which PR it applies to: 1. Start with airflow 1.10.1 in a fresh install 2. Run `AIRFLOW__WEBSERVER__RBAC=true airflow initdb` 3. Run `airflow create_user --username op --password op --role Op --firstname op@localhost --lastname "-" --email "op@localhost"` 4. Log in with op:op 5. You should be able to pause/unpause a DAG (Refresh to check, or look at the status of the request in the browser inspector as the checkbox is updated irrespective of any error returned) 6. upgrade to 1.10.2 (including running `airflow upgradedb`) 7. Try pausing/unpausing again. Check the status - for me it failed. For me after upgrading to 1.10.2 pause doesn't work and I get a 302 to `/login` instead. Given it's sometime between 1.10.1 and 1.10.2 it's probably not related to this PR. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details
ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details URL: https://github.com/apache/airflow/pull/4569#issuecomment-469779758 I'm trying to debug something - I've gone from a 1.10.2 RBAC install to master, and I'm no longer able to pause DAGs Viewer shouldn't be able to _edit_ anything though, right? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details
ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details URL: https://github.com/apache/airflow/pull/4569#issuecomment-469754091 Am I reading this right - does it give Viewer role the ability to edit dags. That seems ... incorrect? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [airflow] ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details
ashb commented on issue #4569: [AIRFLOW-3745] Fix viewer not able to view dag details URL: https://github.com/apache/airflow/pull/4569#issuecomment-469743622 Am I reading this right - does it give Viewer role the ability to edit dags. That seems incorrect? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
