You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "guan404ming (via GitHub)" <gi...@apache.org> on 2025/08/21 14:29:04 UTC
[PR] Allow preloaded options and params through search params [airflow]
guan404ming opened a new pull request, #54783:
URL: https://github.com/apache/airflow/pull/54783
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
## Related PR
#54506
cc @Lee-W
## How
- Enables HITL forms to be preloaded with default options and parameters via URL search parameters
```url
.../required_actions?options=["option 1","option 2"]¶ms={"key":"value"}
```
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3217972249
> @guan404ming / @Lee-W thanks for the discussion! My concerns are fixed by the rework and looks very good for me!
>
> > I probably need to check whether , exists in each option from the backend?
>
> That is really a limitation... that would be a + for having JSON support... mhm. I'd think it would be OK to assume no `,` is within the options and if somebody complaints then it can be addressed also in future. Would be really a nice case in my view.
>
> One option as resolution could be to offer to support JSON as well as plain text style... but might be un-needed complexity if nobody is using/needing it. Would make it easy for users to consume and still would leave the option for the nice cases... I would defer this to a future if really somebody needs it.
Thanks for jumping in on the discussion and helping us improve this feature! ๐
I totally agree that restricting `_options` and excluding `,` makes sense. If anyone really needs these, we can open a new discussion about whether we want to go for it and how to make it happen.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "guan404ming (via GitHub)" <gi...@apache.org>.
guan404ming commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3217976802
> I feel we probably need a Respond button for cases that have a default (like this one). Otherwise, users like me might be confused and don't know what to do next ๐ค Not a blocker though.
Here is the version with a respond button and I think I don't have strong opinion here since both of them look good to me. First one persist the consistent one-click ux and second one make the user flow more natural. What do you think?
https://github.com/user-attachments/assets/9a28ef1d-c861-4402-ade2-e816bec61cdc
> I totally agree that restricting _options and excluding , makes sense. If anyone really needs these, we can open a new discussion about whether we want to go for it and how to make it happen.
Agree with this.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3217968803
@guan404ming / @Lee-W thanks for the discussion! My concerns are fixed by the rework and looks very good for me!
> I probably need to check whether , exists in each option from the backend?
That is really a limitation... that would be a + for having JSON support... mhm. I'd think it would be OK to assume no `,` is within the options and if somebody complaints then it can be addressed also in future. Would be really a nice case in my view.
One option as resolution could be to offer to support JSON as well as plain text style... but might be un-needed complexity if nobody is using/needing it. Would make it easy for users to consume and still would leave the option for the nice cases... I would defer this to a future if really somebody needs it.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "guan404ming (via GitHub)" <gi...@apache.org>.
guan404ming commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3213064908
> Not sure whether it's ideal in this case as we have both options and params here. If we do things this way, it will be confusing when users want something like params_input={"options": ...} (it probably doesn't make as much sense but should be valid)
I also have the concern about that case. Since we already preserve the `options`, it would cause conflict like @Lee-W said if we use `?key=value` here.
IMO The preload behavior in this PR is scoped only to `HitlResponseForm` and wonโt affect other forms. In this sense, `options` and `params` could remain preserved specifically for hitl without impacting other pages (once we implement additional preload behaviors later). Other pages could still use `http://.../required_actions?key=value` to preload data
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3218013995
> Here is the version with a respond button for single option preload case and I think I don't have strong opinion here since both of them look good to me. First one persist the consistent one-click ux and second one make the user flow more natural. What do you think?
I am not sure about the implementation complexity that you need to have the "single option" then in a dropdown and having a respond button... smells a bit more complex. Also UX would be a nit in-consistent. No strong feeling on this but I feel the previous way that still 3 buttons are presented but the pre-select is highlighted is a nit more consistent. Especially looking at a "Reject"/"Approve" scenario (Where in the current HITL Example because of the timeout the Reject is always proposed as default, if you post a link with a 2-click Approve option then approve would be highlighted.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3217955687
> 1. Single option preloading (`?_options=option 1`)
>
>
> Screen.Recording.2025-08-24.at.3.55.55.PM.mov
I feel we probably need a `Respond` button for cases that have a default (like this one). Otherwise, users like me might be confused and don't know what to do next ๐ค Not a blocker though.
> 2. Multiple option preloading (`?_options=option 1,option 2`)
>
>
> Screen.Recording.2025-08-24.at.3.54.40.PM.mov
I probably need to check whether `,` exists in each option from the backend?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3212934964
> Thanks for adding this feature. That totally makes sense. Just I am not 100% good with the choice of implementation. Similar we had in Airflow 2.x for the trifger form (and I just noticed that this feature is not implemented in 3.x... so need to clean docs...) - if you take alook to [airflow.apache.org/docs/apache-airflow/stable/core-concepts/params.html](https://airflow.apache.org/docs/apache-airflow/stable/core-concepts/params.html):
>
> > To pre-populate values in the form when publishing a link to the trigger form you can call the trigger URL /dags/<dag_name>/trigger and add query parameter to the URL in the form name=value, for example /dags/example_params_ui_tutorial/trigger?required_field=some%20text. To pre-define the run id of the DAG run, use the URL parameter run_id.
>
> Can you make it for the feature here the same, so that no `params={...}` need to be used but you can directly call via `http://.../required_actions?key=value`?
Not sure whether it's ideal in this case as we have both `options` and `params` here. If we do things this way, it will be confusing when users want something like `params_input={"options": ...}` (it probably doesn't make as much sense but should be valid)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3217068427
> The users still need to click the `Respond` button, but not necessarily the options.
>
> > Still if mis-understood the "options", we could make this a special field naming like "_options" and/or reserve the key such that if the form has the same it is just not possilbe to pre-populate. Even if this is a restriction it is much easier to be able to pass values to the form w/o the need to build and envode a JSON dict in a URL as param value.
>
> I don't think I have enough frontend knowledge to judge whether decoding a json dict from a frontend perspective is complicated, so I'll leave this to @guan404ming . From the backend perspective, there will be a utility method once this is merged, so users typically don't need to deal with JSON encoding. If we decide to go with `_options`, I'll add another check to ensure this unusual key is not used.
Ah, okay, so passing the "options" would be only beneficial if multiple options should be pre-selected, as single options would render as a button directly.
My argument is mostly not about the decoding (this can be done as code, I am sure this is not a big challenge) but about the complexity somebody need to invest to create a URL with the right parameters to pre-select. Making a URL with `.../respond?key=value` is much easier than `.../respond?params={"key":"value"}` which would need to properly URLencoded to `.../respond?params=%7B%22key%22%3A%22value%22%7D` and if it is not working like expected then it is very hard to debug.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3221281037
I think a simple `encodeURIComponent(JSON.stringify(options))` is the simplest option and leaves less risk to use parsing it wrong. URL readability isn't a big concern here. And when filling out the form, we simply ignore any option which doesn't fit a dropdown.
We should probably have a "Submit" button on more forms. I agree that "Approve" should probably get the default main button styling.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3215403851
I do not understand your concerns here. (I mean the technical reason, maybe due to mis-understanding) - can you elaborate the reason a bit more?
I would propose to make parameter input passing consistent to minimize mental load, encoding a params dict in a single value is a lot of mental load if somebody wants to prepare a URL, also quotes and special chars need to be encoded.
besides this proper docs are needed describing this to users as interface, else somebody need to be expert in source tree in order to make a parameterized call.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3217075776
> > The users still need to click the `Respond` button, but not necessarily the options.
> > > Still if mis-understood the "options", we could make this a special field naming like "_options" and/or reserve the key such that if the form has the same it is just not possilbe to pre-populate. Even if this is a restriction it is much easier to be able to pass values to the form w/o the need to build and envode a JSON dict in a URL as param value.
> >
> >
> > I don't think I have enough frontend knowledge to judge whether decoding a json dict from a frontend perspective is complicated, so I'll leave this to @guan404ming . From the backend perspective, there will be a utility method once this is merged, so users typically don't need to deal with JSON encoding. If we decide to go with `_options`, I'll add another check to ensure this unusual key is not used.
>
> Ah, okay, so passing the "options" would be only beneficial if multiple options should be pre-selected, as single options would render as a button directly.
From what I last discussed with @guan404ming , single options will work this way (like multiple options)if users are using this URL this way.
> My argument is mostly not about the decoding (this can be done as code, I am sure this is not a big challenge) but about the complexity somebody need to invest to create a URL with the right parameters to pre-select. Making a URL with `.../respond?key=value` is much easier than `.../respond?params={"key":"value"}` which would need to properly URLencoded to `.../respond?params=%7B%22key%22%3A%22value%22%7D` and if it is not working like expected then it is very hard to debug.
My issue is that `_options`, `params.key1`, and `params.key2` are not logically on the same level. But `7B%22key%22%3A%22value%22%7D` also seems to be a valid concern. As the latter might cause more trouble , we probably could go with `?_options=...&key1=..`.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "guan404ming (via GitHub)" <gi...@apache.org>.
guan404ming commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3217154737
| Approach | Pros | Cons | Viewpoint |
|----------|------|------|-----------|
| **`...?params={...}` (JSON encoding)** | - Clear structure (no conflict between params and options)<br>- Good extensibility | - Requires JSON encode/URL encode, hard to write and debug<br>- Higher cognitive load for users | Concern about `options` vs `params` conflict |
| **`...?_options=...&key=value` (flat query)** | - Intuitive syntax, consistent with Airflow 2.x<br>- No encoding needed, easy to debug<br>- Lower cost for users to prepare URLs | - `_options` and `params` are not on the same logical level<br>- Need to reserve `_options` to avoid conflicts with normal keys | Preference for simplicity, consistency, and usability |
I created this table to summarize the discussion. Personally, I find Jens's concern reasonable if we try to think from userโs perspective. The flat query style feels more comfortable and approachable. Especially since some users of this feature may not be technical. I think reserving `_options` is a suitable solution to balance usability with potential conflicts where Wei and I concern about.
> From what I last discussed with @guan404ming, single options would behave like multiple options if users pass them through the URL.
I initially implemented it this way. However, after manual testing, I found it was not consistent with our original one-click design in single option. Therefore, I switched to preloading with default options and I think it would be more consistent and user-friendly. Next, I will adjust the preload implementation and update the demo with screenshots covering:
1. Single option preloading
2. Multiple option preloading
3. Key-value pair params
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3211939985
Thanks for adding this feature. That totally makes sense. Just I am not 100% good with the choice of implementation. Similar we had in Airflow 2.x for the trifger form (and I just noticed that this feature is not implemented in 3.x... so need to clean docs...) - if you take alook to https://airflow.apache.org/docs/apache-airflow/stable/core-concepts/params.html:
> To pre-populate values in the form when publishing a link to the trigger form you can call the trigger URL /dags/<dag_name>/trigger and add query parameter to the URL in the form name=value, for example /dags/example_params_ui_tutorial/trigger?required_field=some%20text. To pre-define the run id of the DAG run, use the URL parameter run_id.
Can you make it for the feature here the same, so that no `params={...}` need to be used but you can directly call via `http://.../required_actions?key=value`?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "guan404ming (via GitHub)" <gi...@apache.org>.
guan404ming commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3228540196
> I think a simple encodeURIComponent(JSON.stringify(options)) is the simplest option and leaves less risk to use parsing it wrong. URL readability isn't a big concern here. And when filling out the form, we simply ignore any option which doesn't fit a dropdown.
Agree with making it simple, I think it would be easy for user to prepare it after we got the utility function to generate url with preload options.
Thus currently the url with preloaded data would be
```
...?_options=["option 1", "option 2"]&key=value
```
> We should probably have a "Submit" button on more forms. The number of clicks is less important than it being clear what you're about to do. I agree that "Approve" should probably get the default main button styling.
Nice catch, I like this perspective to keep the safety on the first place. I've update the render logic like:
- show option button with preloaded option highlighted in Approval case (preloaded with "option 4" and "option 5")
<img width="1920" height="1182" alt="image" src="https://github.com/user-attachments/assets/ce589f2d-7d34-4fea-a30f-65a6a0b824d9" />
- show option dropdown and respond button in the other cases (preloaded with "Approve")
<img width="1920" height="1200" alt="image" src="https://github.com/user-attachments/assets/acd180e2-acfd-4b16-a8a6-f1668ec0d6ac" />
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "guan404ming (via GitHub)" <gi...@apache.org>.
guan404ming commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3217927551
Two update in url
1. use `_options` for preload options and flat key-value pair for preload params in hitl
2. change supported list format from `_options=["option 1", "option 2"]` to `_options=option 1, option 2` (avoid to use json-like format to make it more user friendly)
```url
.../required_actions?_options=option 1,option 2&key1=value1&key2=value2...
```
1. Single option preloading (`?_options=option 1`)
https://github.com/user-attachments/assets/86308fdc-e399-4386-9162-17b1d320fac8
3. Multiple option preloading (`?_options=option 1,option 2`)
https://github.com/user-attachments/assets/2e99c2ee-b769-4202-aede-b7643b1fae52
4. Key-value pair params (`?information=test`)
https://github.com/user-attachments/assets/af940693-7156-405a-b838-2632370dd151
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3218047215
No strong opinion either. Then I'll let @guan404ming decide what's the best ๐
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "guan404ming (via GitHub)" <gi...@apache.org>.
guan404ming commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3218107829
> I am not sure about the implementation complexity that you need to have the "single option" then in a dropdown and having a respond button... smells a bit more complex.
The implementation itself wouldnโt be too complex, since our FlexibleForm can render fields based on the passed value type.
However, the logic feels a bit unintuitive and could be harder to maintain.
> No strong opinion either. Then I'll let @guan404ming decide what's the best ๐
I would personally prefer to stick with the current approach (highlight the preload options) for now, but we can revisit and adjust later based on user feedback.
Thanks for all discussion to make this feature better! ๐
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3217063313
> > > I do not understand your concerns here. (I mean the technical reason, maybe due to mis-understanding) - can you elaborate the reason a bit more?
> > > I would propose to make parameter input passing consistent to minimize mental load, encoding a params dict in a single value is a lot of mental load if somebody wants to prepare a URL, also quotes and special chars need to be encoded.
> > > besides this proper docs are needed describing this to users as interface, else somebody need to be expert in source tree in order to make a parameterized call.
> >
> >
> > The main reason is that we're not just adding parameter input, but also the option we want to preload. If there's only parameter input, I'd say we should go with `key=value...`. But now we have 2 things to add, `options` and `params`, and `options` might be a valid key for `params`. If we receive `options=...`, should we view it as a `options` or show it be part of `params`
>
> Mhm, I do not understand about the "options" - this is what the user needs to confirm anyway? We decided not making a 1-click solution so the "2 click solution" here with pre-populated fields would anyway require to select the user to click on "option1", "option2", "option3" manually. What would be pre-selectable for the user with passing the "option"? Is this not the core idea of the second click the user needs to make?
The demo video seems to miss the "options" part. But the idea is that whenever we receive `?...=...&...=...` we'll render the page like multiple options and have the options pre-loaded.
<img width="1049" height="425" alt="image" src="https://github.com/user-attachments/assets/79dd17ba-2564-476b-9e5b-97a35248a8f9" />
The users still need to click the `Respond` button, but not necessarily the options.
> Still if mis-understood the "options", we could make this a special field naming like "_options" and/or reserve the key such that if the form has the same it is just not possilbe to pre-populate. Even if this is a restriction it is much easier to be able to pass values to the form w/o the need to build and envode a JSON dict in a URL as param value.
I don't think I have enough frontend knowledge to judge whether decoding a json dict from a frontend perspective is complicated, so I'll leave this to @guan404ming . From the backend perspective, there will be a utility method once this is merged, so users typically don't need to deal with JSON encoding. If we decide to go with `_options`, I'll add another check to ensure this unusual key is not used.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3217002203
> > I do not understand your concerns here. (I mean the technical reason, maybe due to mis-understanding) - can you elaborate the reason a bit more?
> > I would propose to make parameter input passing consistent to minimize mental load, encoding a params dict in a single value is a lot of mental load if somebody wants to prepare a URL, also quotes and special chars need to be encoded.
> > besides this proper docs are needed describing this to users as interface, else somebody need to be expert in source tree in order to make a parameterized call.
>
> The main reason is that we're not just adding parameter input, but also the option we want to preload. If there's only parameter input, I'd say we should go with `key=value...`. But now we have 2 things to add, `options` and `params`, and `options` might be a valid key for `params`. If we receive `options=...`, should we view it as a `options` or show it be part of `params`
Mhm, I do not understand about the "options" - this is what the user needs to confirm anyway? We decided not making a 1-click solution so the "2 click solution" here with pre-populated fields would anyway require to select the user to click on "option1", "option2", "option3" manually. What would be pre-selectable for the user with passing the "option"? Is this not the core idea of the second click the user needs to make?
Still if mis-understood the "options", we could make this a special field naming like "_options" and/or reserve the key such that if the form has the same it is just not possilbe to pre-populate. Even if this is a restriction it is much easier to be able to pass values to the form w/o the need to build and envode a JSON dict in a URL as param value.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on PR #54783:
URL: https://github.com/apache/airflow/pull/54783#issuecomment-3216110523
> I do not understand your concerns here. (I mean the technical reason, maybe due to mis-understanding) - can you elaborate the reason a bit more?
>
> I would propose to make parameter input passing consistent to minimize mental load, encoding a params dict in a single value is a lot of mental load if somebody wants to prepare a URL, also quotes and special chars need to be encoded.
>
> besides this proper docs are needed describing this to users as interface, else somebody need to be expert in source tree in order to make a parameterized call.
The main reason is that we're not just adding parameter input, but also the option we want to preload. If there's only parameter input, I'd say we should go with `key=value...`. But now we have 2 things to add, `options` and `params`, and `options` might be a valid key for `params`. If we receive `options=...`, should we view it as a `options` or show it be part of `params`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow preloaded hitl options and params through search params [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W merged PR #54783:
URL: https://github.com/apache/airflow/pull/54783
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org