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

[AIRFLOW-5634] Don't allow editing of DagModelView #6308

Merged
merged 1 commit into from
Oct 14, 2019

Conversation

ashb
Copy link
Member

@ashb ashb commented Oct 11, 2019

Jira

Description

  • Most/all of these fields are not fur user editing, with the exception of
    Pause which is set in other ways, and if a user edits these it will just
    confuse the system.

    So lets disable the FAB edit form.

    (Not to mention that the form was broken because it didn't accept the
    last_scheduler_run as filled out.)

    If anyone feels this is worth keeping I can make the it work but not allow editing of most/all fields

@ashb ashb requested review from feluelle and kaxil October 11, 2019 19:20
@ashb
Copy link
Member Author

ashb commented Oct 11, 2019

Similar to #6307 but for the RBAC UI this time (and I took a different approach here)

feluelle
feluelle previously approved these changes Oct 12, 2019
tests/www/test_views.py Outdated Show resolved Hide resolved
tests/www/test_views.py Show resolved Hide resolved
@feluelle feluelle dismissed their stale review October 12, 2019 12:34

"Requesting Changes"

Copy link
Member

@feluelle feluelle left a comment

Choose a reason for hiding this comment

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

Sorry, I wanted to choose "Request changes"

@feluelle
Copy link
Member

I think editing dag runs would then be a different ticket/PR.

Most/all of these fields are not fur user editing, with the exception of
Pause which is set in other ways, and if a user edits these it will just
confuse the system.

So lets disable the FAB edit form.

(Not to mention that the form was broken because it didn't accept the
last_scheduler_run as filled out.)
@ashb ashb requested a review from feluelle October 14, 2019 09:37
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@8cbfd93). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #6308   +/-   ##
=========================================
  Coverage          ?   80.34%           
=========================================
  Files             ?      616           
  Lines             ?    35733           
  Branches          ?        0           
=========================================
  Hits              ?    28710           
  Misses            ?     7023           
  Partials          ?        0
Impacted Files Coverage Δ
airflow/www/views.py 75.19% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8cbfd93...edbb4d9. Read the comment docs.

@ashb ashb merged commit c082065 into apache:master Oct 14, 2019
@ashb ashb deleted the disable-dagmodelview-edit branch October 14, 2019 11:40
ashb added a commit to ashb/airflow that referenced this pull request Oct 14, 2019
Most/all of these fields are not fur user editing, with the exception of
Pause which is set in other ways, and if a user edits these it will just
confuse the system.

So lets disable the FAB edit form.

(Not to mention that the form was broken because it didn't accept the
last_scheduler_run as filled out.)

(cherry picked from commit c082065)
adityav pushed a commit to adityav/airflow that referenced this pull request Oct 14, 2019
Most/all of these fields are not fur user editing, with the exception of
Pause which is set in other ways, and if a user edits these it will just
confuse the system.

So lets disable the FAB edit form.

(Not to mention that the form was broken because it didn't accept the
last_scheduler_run as filled out.)

(cherry picked from commit c082065)
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.

3 participants