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

DAG triggering page does not reflect conf parameter in the URL query string #32859

Closed
2 tasks done
jsgh7276 opened this issue Jul 26, 2023 · 7 comments · Fixed by #37497
Closed
2 tasks done

DAG triggering page does not reflect conf parameter in the URL query string #32859

jsgh7276 opened this issue Jul 26, 2023 · 7 comments · Fixed by #37497
Assignees
Labels
affected_version:main_branch Issues Reported for main branch affected_version:2.6 Issues Reported for 2.6 area:UI Related to UI/UX. For Frontend Developers. kind:bug This is a clearly a bug priority:low Bug with a simple workaround that would not block a release

Comments

@jsgh7276
Copy link

Apache Airflow version

2.6.3

What happened

I've found some unexpected behaviors of airflow webserver UI, when visiting DAG triggering page with conf in the URL query string.

  1. conf in the URL is not applied to the triggering page param text input.
  2. Cannot edit the actual DagRun conf when I trigger the DAG from triggering page with conf in the URL query string.

I have this simple DAG below, and I wanted to guide coworkers to trigger this DAG with specific conf included.

with DAG(
        dag_id="example_dag",
        default_args={
            "depends_on_past": False,
            "wait_for_downstream": False,
            "email_on_failure": False,
            "email_on_retry": False
        },
        description="This is example dag",
        start_date=datetime(2023, 6, 2),
        schedule=None,
        catchup=False,
        params={
            "key1": "default value 1",
            "key2": "default value 2"
        }
) as dag:

    t1 = BashOperator(
        task_id="example_task",
        bash_command="echo hello"
    )

    t1

So I provided the triggering URL http://webserver.url:8080/trigger?dag_id=example_dag&conf={"key1":"value1","key2":"value2"}.
But when visiting the URL, UI does not reflect the passed conf parameter in the query string.

As you can see in the screenshot below, pre-populated texts are just showing default params of the DAG, not the conf in the URL query.
image

Moreover, when I trigger the DAG by clicking the blue button in the above page, that DagRun gets conf in the URL query string. In other words, text input in the triggering page does not affect DagRun when the user visit the page with conf query string included. (This problem was also reproduced on airflow version 2.5.2)

What you think should happen instead

I suggest the text input in triggering page should reflect the passed conf in the query string.
Also, when the DagRun is triggered at the triggering page where conf is included in URL, I think DagRun should utilize conf in the text input, not in the URL.

How to reproduce

It is reproducible with above DAG example and the example URL(http://webserver.url:8080/trigger?dag_id=example_dag&conf={"key1":"value1","key2":"value2"}).

Operating System

CentOS Linux, MacOS

Versions of Apache Airflow Providers

apache-airflow-providers-common-sql==1.5.1
apache-airflow-providers-ftp==3.4.1
apache-airflow-providers-http==4.4.1
apache-airflow-providers-imap==3.2.1
apache-airflow-providers-mysql==4.0.2
apache-airflow-providers-sqlite==3.4.1

Deployment

Virtualenv installation

Deployment details

No response

Anything else

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@jsgh7276 jsgh7276 added area:core kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet labels Jul 26, 2023
@boring-cyborg
Copy link

boring-cyborg bot commented Jul 26, 2023

Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.

@hussein-awala
Copy link
Member

It should be fixed by #31583 which will be released in 2.7.0, I tested with 2.6.3 and I had the same issue, but I couldn't reproduce it on main.

@jscheffl
Copy link
Contributor

I don't remember instantly but I think other than a JSON there was an option to pass guided defaults via the URL to the form. I#d need to check but I remember I had that working.
If params in URL are overriding the form values, I'd rather think this would be a bug. I'd try to make a test, then it might be valid to re-open.

@jsgh7276
Copy link
Author

jsgh7276 commented Jul 27, 2023

@hussein-awala
I've tested the problem on breeze dev environment (2.7.0.dev0) but it is still reproducing with brand new commits(3237cb3). Can you please check it again?

@jsgh7276
Copy link
Author

jsgh7276 commented Jul 27, 2023

@jens-scheffler-bosch
According to the code, these are available options of trigger page URL: run_id, origin, unpause, conf, execution_date. I could not find another option to pass guided defaults.

Thanks, please reopen this if the overriding bug is reproduced.

@jscheffl
Copy link
Contributor

Sorry @jsgh7276 that my response took longer. I did a explicit test and can confirm actually that this is a bug. If you pass the confparameter in the URL this is always picked as being the form submission irrespective of what form data was submitted by the user. Also form fields are not pre-populated.

So probably it is worth fixing this and I can see the use case of passing a pre-population URL to users. But more I see the bug of form values being ignored if pre-populated conf is used.

I had parts of this already fixed and re-factored the code behind already, waiting to get PR #31301 being merged, from there I assume it would be good to develop a fix. Current code is unfortunately a bit spaghetti, looking forward to have the other PR merged, then I'd be happy to prepare a fix. (I'd like to prevent applying a fix before merge of #31301 as existing code is bad and I needed to re-work my PR already multiple times)

@jscheffl jscheffl reopened this Jul 31, 2023
@jscheffl jscheffl added priority:low Bug with a simple workaround that would not block a release area:UI Related to UI/UX. For Frontend Developers. affected_version:main_branch Issues Reported for main branch affected_version:2.6 Issues Reported for 2.6 and removed area:core needs-triage label for new issues that we didn't triage yet labels Jul 31, 2023
@jscheffl jscheffl self-assigned this Jul 31, 2023
@jsgh7276
Copy link
Author

jsgh7276 commented Aug 1, 2023

I understood what you are considering now and I’ll happily wait for the fix. Thank you so much for your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected_version:main_branch Issues Reported for main branch affected_version:2.6 Issues Reported for 2.6 area:UI Related to UI/UX. For Frontend Developers. kind:bug This is a clearly a bug priority:low Bug with a simple workaround that would not block a release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants