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

Pipelines: Server side deployment state tracking #2283

Merged
merged 5 commits into from
Jun 27, 2023

Conversation

Pezmc
Copy link
Contributor

@Pezmc Pezmc commented Jun 14, 2023

Description

Split into two parts:

  • Refactor the existing state tracking to work based on stage deploy state (deduced from instance state)
  • Move storage of instance deploy state server side

Key advantages:

  • State persists across page reloads
  • Deploy complete alert works regardless of what starting state the instance was in
Screen.Recording.2023-06-14.at.15.28.35.mov

Related Issue(s)

N/a

Checklist

  • I have read the contribution guidelines
  • [-] Suitable unit/system level tests have been added and they pass - Not really clear how to test this. Page already has E2E coverage.
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on flowforge/helm to update ConfigMap Template
    • Issue/PR raised on flowforge/CloudProject to update values for Staging/Production

Labels

  • Backport needed? -> add the backport label
  • Includes a DB migration? -> add the area:migration label

@Pezmc
Copy link
Contributor Author

Pezmc commented Jun 14, 2023

@joepavitt Would appreciate some click testing from you to verify the UI still behaves as you expect!

@joepavitt
Copy link
Contributor

Didn't work for existing pipeline I had, but creating a new pipeline worked at first whilst in "installing", but my local seems to be getting stuck in "starting" again.

Also notice that the polling doesn't seem to be enabled upon refresh? If I click "play" and then immediately refresh, has some instances where the target stage stay locked in "Installing" until I refreshed again.

@Pezmc
Copy link
Contributor Author

Pezmc commented Jun 15, 2023

@joepavitt Looks like I hadn't pushed my latest 🤦 , will do a sanity check and retag you! Sorry for wasted time!

This shouldn't happen, but the page shouldn't break if it does
@Pezmc
Copy link
Contributor Author

Pezmc commented Jun 19, 2023

@joepavitt Ready for a re-review

Copy link
Member

@knolleary knolleary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done some clicking around, all appears functional. Refreshing the page, getting the inflight state updated properly. All good.

@knolleary
Copy link
Member

The only thing that caught me out was what lead me to raise #2332 as I thought things weren't updating properly. But that's (quite literally) a separate issue.

@joepavitt
Copy link
Contributor

Have managed to get my local up and running again for a couple of days blocked. Will try and get to this review this afternoon as I'm trying to round up the Dashboard work. If Nick is happy though, feel free to proceed @Pezmc

@joepavitt joepavitt merged commit 723174f into main Jun 27, 2023
4 checks passed
@joepavitt joepavitt deleted the feat-deloyment-state-tracking branch June 27, 2023 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants