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

Feat/websocket running status FRT-390 #120

Merged
merged 8 commits into from Jun 13, 2022

Conversation

bdebon
Copy link
Contributor

@bdebon bdebon commented Jun 7, 2022

What does this PR do?

Adding websocket running status feature to show all the chip with correct color and running status

> Link to the JIRA ticket

List of things done and complex due to the way these websockets work:

  • Matching small id (eg z23oiuio0) and long uuid. Ids of app / database / environment may vary between the websocket message and their id given by the rest API. The rest API will always send our entities with their uuid. In the websocket, sometimes we have a short version of the id and this version begins with a z. We had to create a utility to match the short and long one.
  • Opening one websocket per cluster on an organization. There is no websocket that allows you to get all your environments in a same call. You have to call a websocket per cluster and then save the different env / application and databases in your store
  • Websockets return env, application, database status only if there is a known status for them. If a status becomes STOPPED or Unknown, the status for the app/database/env won't be in the websocket response anymore. Which means that if you had for example a RUNNING status for an app and then suddenly, the status is STOPPED or Unknown, the app won't be in the websocket answer anymore so you can't update it in your store to change the status from Running to Stopped or Unknown. As a result, everytime we have an websocket message, I first force all app/database/env to set their status to null before updating their new value if it is present in the new message. If you don't do this, your app won't be updated at all and will keep the Running Status.
  • Because of the previous point, we had another corner case to handle. When we have different cluster, we have different websockets. If we use the previous point only, each message from each socket will reset the other services and environment. To put simply, if you have an environment on one cluster, if the first websocket set its running status to RUNNING, the next websocket message from the other cluster will reset its running status to null. We have to reset the status only if the services or env belongs to the cluster... That's another tricky check that we have to do on our side
  • Created the interfaces for the websocket answer because the Websocket model is not documented in the api-doc so it's not part of the generated api.
  • Made sure that sockets are closed everytime their are not relevant anymore. In your case, everytime we change orga, we need to close the sockets for the clusters of this past organization.
  • Made sure to auto reconnect the websocket if there is no data in the first answer. The way this websocket works is, if a cluster has nothing to answer, no running status to display, it answers nothing, then the websocket is automatically closed so we have to retry some time to time in case the cluster would have finally something to say
  • Database plugged
  • Made the difference between database managed and contained (managed are not handled by the websocket and we have to use normal status for them)

todo:

  • Performances issues with useEffect that goes in infinite loop if we do what the linter says
  • Testing the functions that I have put in useCallback

bugs:

As opposed to v2 websocket, we tried here to call only at parent level. On the Layout component. The thing is that the socket answer with everything but the data are possibly not yet in the store... So if the message of the socket has passed and there is no new message when we fetch the data, the data will be missing its running status.

This has been resolved by adding in the websocket component in the useEffect dependency array, the applicationLoadingStatus === 'loaded', dbsLoadingStatus === loaded etc...
With that, as soon as we have a loaded status, we replay the useEffect populating, this time, entities that are stored in the slices. Hurray. => Other bug, right now, with the one-to-many relationship, we don't set back the loadingStatus to unloaded. Once we have successfully loaded one collection, the loadingStatus stays forever on loaded... We'll have to address that later.


PR Checklist

Global

  • This PR does not introduce any breaking change
  • This PR introduces breaking change(s) and has been labeled as such
  • I have found someone to review this PR and pinged him

Store

  • This PR introduces new store changes

NX

  • I have run the dep-graph locally and made sure the tree was clean i.e no circular dependencies
  • I have followed the library pattern i.e feature, ui, data, utils

Clean Code

  • I made sure the code is type safe (no any)
  • I have included a feature flag on my feature, if applicable

@nx-cloud
Copy link

nx-cloud bot commented Jun 7, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 71f5331. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

@RemiBonnet
Copy link
Member

A preview environment was automatically created via Qovery.
Click on the link below to follow its deployment and use it.
👉 [PR] staging - Draft: Feat/websocket running status FRT-390 - 2022-06-07T17:17:27Z

@bdebon
Copy link
Contributor Author

bdebon commented Jun 8, 2022

CleanShot 2022-06-08 at 13 41 51@2x

There are some case where the socket return two environments which are in reality the same one. Just one has a short id, and the other has a normal uuid... Another corner case to handle

@bdebon bdebon force-pushed the feat/websocket-running-status-FRT-390 branch 2 times, most recently from 937bcf3 to 175a72b Compare June 8, 2022 14:27
storeDatabasesRunningStatus(message)
}
}
}, [dispatch, lastMessage, appsLoadingStatus, dbsLoadingStatus, envsLoadingStatus, clusterId])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here, eslint says that environmentsAssociatedToCluster is missing from the dependencies. It works very well without it. But as soon as you add it in the dependencies, you get an infinite loop. I tried many different things.
The easiest solution would be to be able to read the env slice from within the useEffect but I can't use useSelector inside a useEffect. I don't know what I do wrong here and it's pretty hard to formulate the issue to Google. I tried for hours...

@bdebon
Copy link
Contributor Author

bdebon commented Jun 9, 2022

CleanShot 2022-06-08 at 13 41 51@2x

There are some case where the socket return two environments which are in reality the same one. Just one has a short id, and the other has a normal uuid... Another corner case to handle

This still need to be fixed either from backend or on our side if backend can't change it for whatever reason... The fix frontend side is rather ugly and I would prefer not to implement it if backend can fix that for us.

@bdebon bdebon changed the title Draft: Feat/websocket running status FRT-390 Feat/websocket running status FRT-390 Jun 9, 2022
@bdebon bdebon force-pushed the feat/websocket-running-status-FRT-390 branch from 342d159 to 3574c3a Compare June 13, 2022 09:57
@bdebon bdebon force-pushed the feat/websocket-running-status-FRT-390 branch from 6173e76 to 71f5331 Compare June 13, 2022 14:35
@bdebon bdebon merged commit c6b59e5 into staging Jun 13, 2022
@bdebon bdebon deleted the feat/websocket-running-status-FRT-390 branch June 13, 2022 14:45
@bdebon
Copy link
Contributor Author

bdebon commented Jun 22, 2022

🎉 This PR is included in version 1.0.0-staging.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@bdebon
Copy link
Contributor Author

bdebon commented Nov 17, 2022

🎉 This PR is included in version 1.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants