Bug #15672
closedList of subprocesses limited in process view
Added by Moritz Gilsdorf about 5 years ago. Updated 8 months ago.
Description
In order to make large numbers of subprocesses readily available and easily navigable to users:
- Switch to using the DataExplorer WB2 component.
- Make filtering on state mutually exclusive and use the built in filtering of the DataExplorer. This will require changing the column filter pop up to be radio buttons.
- Remove the toggles from the subprocess panel, but keep the data about the number of subprocesses in each state.
Files
Screenshot 2019-09-30 at 10.44.03.png (286 KB) Screenshot 2019-09-30 at 10.44.03.png | Moritz Gilsdorf, 09/30/2019 08:44 AM |
Updated by Moritz Gilsdorf about 5 years ago
There seems to be a limit in how many sub-processes are listed in the process view. In the workflow run shown in the attached screenshot are in total 11169 steps but the api seems to only fetch the first 1000.
!Screenshot 2019-09-30 at 10.44.03.png!
Updated by Tom Morris about 5 years ago
- Target version changed from Workbench2 Q3, Q4 to 2019-11-06 Sprint
Updated by Peter Amstutz about 5 years ago
- Related to Feature #13327: More responsive page loads with API ?include=container_uuid added
Updated by Eric Biagiotti about 5 years ago
- For listings, use posts with _method=GET so we won't get a 414 and load them into state.
- Test performance with 11k+ entries.
- If necessary add virtualization to the grid.
Updated by Eric Biagiotti about 5 years ago
- Status changed from New to In Progress
Updated by Eric Biagiotti about 5 years ago
- Target version changed from 2019-11-06 Sprint to 2019-11-20 Sprint
Updated by Eric Biagiotti about 5 years ago
I am currently working on adding a data explorer to the process/subprocess view (the notion of a process is meant to simplify the container request and container relationship in WB2). The data explorer works by using the list API to only get the data that it needs (# of rows being displayed). Normally, it is trivial to filter because you just update the list query.
But, filtering on process status is presenting a problem because it is based on the container and the container request state
attribute (e.g. a process is cancelled if the container request priority is 0, or if the container state is "Cancelled").
Example: I want to display 10 rows or subprocesses excluding ones that are cancelled.
- List and filter container_requests where
priority != 0
, which returns 10 results. - Use the
containerUuid
of the returned container_request objects to get the container (if it exists) and filter wherestate != "Cancelled"
. Lets say this returns 9 results because one container was cancelled. - In this state, we don't have enough data to fill all the rows.
The only solution I can think of is to first get all the subprocesses (container requests where requesting_container_uuid
is the parent container uuid and the corresponding containers if they exists). Then, I can filter them in the code. But depending on the number of subprocesses and the API request limit, this could add many more network requests and kind of defeats the purpose of the data explorer.
Or maybe I am missing something? Let me know if any of this makes sense. Thanks!
Updated by Eric Biagiotti about 5 years ago
A few potential approaches to consider:
Assumptions: All crs have containers. Status filtering operates only on the state/exit_code of the container. Data explorer implementations will have search bars using the trigram index.
Filtering with data explorer- Approach: Get the containerUuid of all the sub-crs that are COMITTED or FINAL (has a container). This requires adding client functionality to bypass API server response limits. Use the list method to get corresponding containers and filter on the containerUuids and "state"/"exit_code" and limit on number of rows to display.
- Pros: Keeps current filtering functionality and adds a data explorer, presenting more useful info to the user.
- Cons: Large network requests, scalability problems.
- Approach: Remove filtering on status and put the data explorer in infinite scroll mode. This adds a button at the bottom of the page that says "Load More" and is more like how WB1 works. This could still result in rendering performance issues unless we block this behind the react-redux update and try using list virtualization.
- Pros: Network friendly and more scalable.
- Cons: Removes functionality from the process page. Could still result in rendering issues with a full list displayed.
New API feature
- Approach: Block this behind adding an API feature to be able to filter container requests by their container's state and exit_code.
- Pros: Network friendly and more scalable. Data explorer will work well and no filtering needs to be done on the client side. Users keep existing filters.
- Cons: API work has to be done first.
Updated by Peter Amstutz about 5 years ago
Eric Biagiotti wrote:
A few potential approaches to consider:
Assumptions: All crs have containers. Status filtering operates only on the state/exit_code of the container. Data explorer implementations will have search bars using the trigram index.
Filtering with data explorerWB1 style with no filtering and infinite scroll
- Approach: Get the containerUuid of all the sub-crs that are COMITTED or FINAL (has a container). This requires adding client functionality to bypass API server response limits. Use the list method to get corresponding containers and filter on the containerUuids and "state"/"exit_code" and limit on number of rows to display.
- Pros: Keeps current filtering functionality and adds a data explorer, presenting more useful info to the user.
- Cons: Large network requests, scalability problems.
- Approach: Remove filtering on status and put the data explorer in infinite scroll mode. This adds a button at the bottom of the page that says "Load More" and is more like how WB1 works. This could still result in rendering performance issues unless we block this behind the react-redux update and try using list virtualization.
- Pros: Network friendly and more scalable.
- Cons: Removes functionality from the process page. Could still result in rendering issues with a full list displayed.
New API feature
- Approach: Block this behind adding an API feature to be able to filter container requests by their container's state and exit_code.
- Pros: Network friendly and more scalable. Data explorer will work well and no filtering needs to be done on the client side. Users keep existing filters.
- Cons: API work has to be done first.
Maybe we should have a quick meeting on this next week but it feels to me like doing it all client side is actually a nontrivial amount of work for an inferior result, whereas if we add an API side query that does exactly what we want (which I believe would be relatively easy), we will also be able to plug it into the existing data table with minimal effort.
Updated by Tom Morris about 5 years ago
- Related to Feature #12917: Support ?include=container_uuid for group contents added
Updated by Tom Morris about 5 years ago
- Related to Idea #15350: [Documentation] Add Python SDK recipe on how to get a container request's real state added
Updated by Eric Biagiotti about 5 years ago
- Target version changed from 2019-11-20 Sprint to 2019-12-04 Sprint
Updated by Eric Biagiotti about 5 years ago
- Blocked by Feature #15854: Add container request filtering based on container.exit_code and container.state added
Updated by Eric Biagiotti about 5 years ago
- Related to Feature #15846: [Workbench2] Data explorer page navigation added
Updated by Eric Biagiotti about 5 years ago
- Related to Feature #15844: [Workbench2] Indicate when a DataExplorer filter is active added
Updated by Eric Biagiotti about 5 years ago
- Target version changed from 2019-12-04 Sprint to 2020-01-02 Sprint
Updated by Eric Biagiotti about 5 years ago
- Docs updated @ commit:d953b5510676f1cb5cfab7a092d2de2cd3c72135 in the Arvados repo.
Latest work:
- Added the ability to create radio button groups for data explorer column filtering.
- Process panel now stores the parent container_request uuid, which is used as a prop to the subprocess panel to populate the dataexplorer.
- Added Run Time and Created At columns. Can't currently sort by runtime.
TODO:
- Add started_at and finished_at time to columns. Users can use the column filtering on the top right of the data explorer to choose what columns to display. Pick a reasonable set of defaults.
- The action triple dot for subprocesses in the data explorer is not set up. Look at project-panel.tsx for an example.
- Search of the subprocess view from the data explorer doesn't work. Use trigram index API call for this.
- Default message on the data explorer for when there are no subprocesses in the view is not being displayed.
- Clicking on a subprocess in the data explorer should update the details panel.
- Navigation when double clicking on a subprocess isn't working.
- Reset the data explorer radio box when navigating to the page?
Updated by Lucas Di Pentima about 5 years ago
- Assigned To changed from Eric Biagiotti to Lucas Di Pentima
Updated by Peter Amstutz about 5 years ago
- Related to Feature #15012: Add /all_processes page added
Updated by Lucas Di Pentima about 5 years ago
Filters were being constructed by doing snake case translation on the attribute name.
Branch 15672-filters-translation-fix
at commit 680b1ffd (wb2 repo) removed this translation and fixes all parts of the code using the FilterBuilder
class.
This is needed on this branch because the filter [['container.state', '=', 'foo']]
gets translated to [['container_state', '=', 'foo']]
.
Also found a similar case on ordering, but that is being type checked against the resources' key names so I think it's not so trivial to fix.
Updated by Peter Amstutz about 5 years ago
Lucas Di Pentima wrote:
Filters were being constructed by doing snake case translation on the attribute name.
Branch15672-filters-translation-fix
at commit 680b1ffd (wb2 repo) removed this translation and fixes all parts of the code using theFilterBuilder
class.This is needed on this branch because the filter
[['container.state', '=', 'foo']]
gets translated to[['container_state', '=', 'foo']]
.Also found a similar case on ordering, but that is being type checked against the resources' key names so I think it's not so trivial to fix.
This LGTM.
Updated by Lucas Di Pentima about 5 years ago
Updates at commit 134e0d36 - branch 15672-subprocess-list-v2
(wb2 repo)
Continued Eric's branch on a separate one, having rebased it to use the previous fixes on filter translation:
- Made status filters added by Eric to be used on the request
- Pagination options now offer 50, 100, 200 and 500 page sizes
- Removed subprocess count and filter sliders panel on the top right corner
- Double click and name-click navigate to the selected subprocess
- Single click updates the detail panel
- When no subprocesses available to list (because of filtering, search, etc) show a message.
- Modified the list api calls to use POST only when the params on the query string are big enough (>1500 chars)
- Added subprocess search by name, not sure if we should be able to search by other properties
Updated by Peter Amstutz almost 5 years ago
Functional comments, haven't looked at the code yet:
Locked and running containers have a runtime of "NaNh NaNm NaNs" (running containers should compute it as now - started_at, Locked should be either blank or zero)
In the "Status filters" popup, clicking on the button works to change the filter. However moving the mouse cursor over the text of an item and clicking it changes the item background but doesn't change the filter. Also it seems unnecessary to select the filter and then separately choose Ok/Cancel, a single click should select the item and close the dialog.
After closing the "Status filters" popup, the "Filters" tooltip remains on the screen, partly obscuring the first row.
The list live-updates the state of individual subprocess items but does not refresh the list. So if you filter on "Running" the items in the list can live-update to "Completed" but are not removed from the list, and new "Running" subprocesses are not added. There's also no button to explicitly refresh the list (this something that is missing from the UI design in general, clicking the item in the breadcrumb bar usually works but is not at all obvious).
Updated by Peter Amstutz almost 5 years ago
Code @ 16860c6f2fedc5c150cab796e93aa21067f28e97
subprocess-panel-actions.ts has a bunch of commented-out code
I see there's a ContainerRunTime component so it shouldn't be too hard to fix the time display (although since 'now' constantly changes, I'm not sure about the preferred way to handle that sort of thing in React, we probably only want update granularity to the nearest minute).
Updated by Peter Amstutz almost 5 years ago
Switching from GET to POST when the URI gets too long -- could we unit test that?
Updated by Peter Amstutz almost 5 years ago
- Related to Bug #15951: Process filtering UI polish added
Updated by Peter Amstutz almost 5 years ago
- Target version changed from 2020-01-02 Sprint to 2020-01-15 Sprint
Updated by Peter Amstutz almost 5 years ago
- Assigned To changed from Lucas Di Pentima to Peter Amstutz
Updated by Peter Amstutz almost 5 years ago
- Target version changed from 2020-01-15 Sprint to 2020-01-29 Sprint
Updated by Peter Amstutz almost 5 years ago
- Assigned To changed from Peter Amstutz to Lucas Di Pentima
Updated by Tom Clegg almost 5 years ago
- Related to Bug #15902: Very slow query for large workflows added
Updated by Lucas Di Pentima almost 5 years ago
Updates at commit 6183e02
- Reviewed Peter's additions and fixed a broken test
- Added a unit test for the list API call with POST method when using big query params as requested on note-30
Updated by Lucas Di Pentima almost 5 years ago
Simplified test code @ commit 22a3f545
Updated by Peter Amstutz almost 5 years ago
Lucas Di Pentima wrote:
Simplified test code @ commit 22a3f545
My only comment would be to recommend having two test cases, one just under the limit (uses GET) and one just over the limit (uses POST), currently only tests the second case one so every query could use POST and this wouldn't detect it.
Rest LGTM, please merge.
Updated by Anonymous almost 5 years ago
- % Done changed from 50 to 100
- Status changed from In Progress to Resolved
Applied in changeset arvados-workbench2|be8de5acc35c47d0b191223e5ece96fcf452ea5d.
Updated by Peter Amstutz almost 4 years ago
- Related to Idea #16945: WB2 Workflows / containers feature parity added
Updated by Peter Amstutz almost 4 years ago
- Related to deleted (Idea #16945: WB2 Workflows / containers feature parity)