Separate action for requesting resource refs. #4211
Conversation
Signed-off-by: Michael Nelson <minelson@vmware.com>
|
Great, thanks for the refactor! |
| @@ -99,11 +104,10 @@ export function getApp( | |||
| installedPackageRef?: InstalledPackageReference, | |||
| ): ThunkAction<Promise<void>, IStoreState, null, AppsAction> { | |||
| return async dispatch => { | |||
| dispatch(requestApps()); | |||
| dispatch(requestApp()); | |||
There was a problem hiding this comment.
Are we going to unify terminology regarding "app" vs "package", etc? I mean, for example this function is called getApp, actions are requestApp and selectApp but it is dealing with *PackageDetail and outputting installedPackageDetail and availablePackageDetail. Or am I not getting the distinction here (which could perfectly be 😅 ) ?
There was a problem hiding this comment.
I guess this is for historical reasons. I did perform a rename from chart to package back in time; however, the apps thing is something still pending.
+1 to move from App to InstalledPackage instead, of course.
There was a problem hiding this comment.
Yes, exactly. I did actually have it with package initially, but then saw the other actions related to App and Apps, and didn't want to create an inconsistency here nor rename all of them here. Happy to do a separate PR with such.
There was a problem hiding this comment.
Great, awesome. This (and the subsequent PRs) is gonna ensure a good UX once we move any big delays to get Refs operation, as the minimal UI still will be rendering and we could possibly add some spinner or whatever to let users know something else is still being fetched.
Thanks!
Signed-off-by: Michael Nelson minelson@vmware.com
Description of the change
Separates out the action for fetching resource refs for an installed application.
Benefits
When the resource refs were first added to the
SelectedAppstate, I think Antonio and I both agreed that it didn't belong there (in theCustomInstalledPackageDetailwhich was temporarily created to accommodate the helm version field), but at the time I wasn't keen to refactor it.Now we have an unrelated issue that for flux (and carvel), the call to fetch the resource refs fails with a 404 initially (within 1 second of creating the installed package) because the underlying reconciliation creates the helm (for flux) release very soon after creating the installed package, but not immediately.
I'm going to create an issue for the plugin backend so that the request for resource refs (a valid one, for which there's already an installed package) doesn't return until the underlying helm release is created (within a second), but am also planning here in the dashboard to work around the issue for now by requesting the resource refs a few times with a fallback (next branch). But to do that, I needed to fix this tech-debt to separate out the call for the resource refs.
Possible drawbacks
Behaviour is the same.
Applicable issues
Additional information