refactor(site): update agent status to include the lifecycle #5835
Conversation
There was a problem hiding this comment.
Very nice job @BrunoQuaresma! I tried it out locally and it looks like the behavior is on-point.
The only comment I have is that visually the "red" for error isn't very noticeable for me (not sure if my slight color blindness plays a role), but the warning color is much more noticeable/full, and the red looks faded (almost gray/pink).
Warning:
Error:
| <StartErrorLifecycle agent={agent} /> | ||
| </Cond> | ||
| <Cond> | ||
| <StartingLifecycle /> |
There was a problem hiding this comment.
This will catch both early created and starting states, so this is fine! In the future we'll have states for shutting down as well.
|
@BrunoQuaresma @kylecarbs @bpmct should we label this as a breaking release? Once this is deployed, and users update to provider v0.6.7+, users will start to see that agents time out during start (default 5 minutes). If they're using startup script to launch a service synchronously, then they'll run into it. (PS. Even if they don't update provider, agent will remain in "starting" state indefinitely which doesn't hamper usage but might be weird.) |
|
Could you post a screenshot of the hover over the icon? I think breaking is alright if we provide some good documentation to fix this, but not if we don't. FWIW we could tell users to use |
|
We definitely want to improve documentation around this, we already mention checking logs @ the end of that troubleshooting link, but we can more explicitly mention the states and what log is relevant. |
|
I think it's bad to break unless we explicitly state that the startup script is taking too long to complete. We can also mention that we broke this in X release, and read the docs here to migrate. |
|
Sure, let's improve the wording and perhaps add a second link 👍🏻. This is mostly a soft-break, though, as the experience is basically the same except for this visual indication here. In the CLI this is going to be basically opt-in by upgrading provider and setting (We could btw use the same property, delay login, for UI feature enabling.) |
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
|
@mafredri about the colors, it is a known issue. I'm going to try to fix that in a next PR. |
…bq/update-agent-status
| @@ -284,65 +280,4 @@ describe("WorkspacePage", () => { | |||
| }) | |||
| }) | |||
| }) | |||
|
|
|||
| describe("Resources", () => { | |||
There was a problem hiding this comment.
I really think tests like these should live in Storybook. Integration tests should be used for user interactions IMO.
BrunoQuaresma
changed the title
Updating the FE, specifically the component to handle the new agent's lifecycles.
Refs: #5749