galaxyproject
Reduce tool form building frequency
Mitigates #18848
When we have multiple data inputs in a tool, the ToolForm will request a tool rebuild for each data input in rapid succession (see https://github.com/galaxyproject/galaxy/issues/18848#issuecomment-2399667355).
This will ensure that only the last request with all the updated data is sent.
Before
After
How to test the changes?
- [ ] I've included appropriate automated tests.
- [x] This is a refactoring of components with existing test coverage.
- [ ] Instructions for manual testing are as follows:
- [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]
License
- [x] I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.
Nothing's really broken here, I would target dev since I could see this having subtle side-effects that we're notoriously bad at testing.
Nothing's really broken here, I would target dev since I could see this having subtle side-effects that we're notoriously bad at testing.
Good point! Done!
We cannot use this.onChange here because on initial load there would be no change in the formData and the onChange event would not be triggered.
Isn't this is a good thing ? What made you decide to revert this ?
Isn't this is a good thing ? What made you decide to revert this ?
That change was breaking this test:
https://github.com/galaxyproject/galaxy/blob/e9b7abfd63d3a5b12bae4d3061bd8a0f03241b52/lib/galaxy_test/selenium/test_workflow_editor.py#L971
Unfortunately, the test results are gone after the rebase. For some reason, the event must be fired on creation for this test to pass (maybe the problem is with the test?). In addition, after intensive debugging, the change didn't seem to make any significant difference in reducing the form rebuilding as far as I could tell, so I decided to revert it. I will restore it and we can investigate the test further.
I don't think request volume / debounce tradeoff is worth it.
Why not, we want the most up-to-date request anyway, no? :thinking:
Sorry, I'm afraid I don't know how to improve the situation :disappointed: I know this is just a palliative measure and the real issue is the complex event interaction in these components.
On the other hand, I could keep debugging and investigating with more time since this is no longer scheduled for this release.
Why not, we want the most up-to-date request anyway, no? 🤔
500ms delay is a lot of delay and a high price to pay.
the change didn't seem to make any significant difference in reducing the form rebuilding as far as I could tell, so I decided to revert it.
This is weird, see https://github.com/galaxyproject/galaxy/issues/18848#issuecomment-2393830406.
I could keep debugging and investigating with more time since this is no longer scheduled for this release.
👍 maybe this is a good moment to see if we want to change how data is passed up and down the component hierarchy.