Fetch workspace classes from server #11571
Conversation
Furisto
force-pushed
the
fo/dynamic-class
branch
4 times, most recently
from
bef547a to
8c45531
Compare
Furisto
force-pushed
the
fo/dynamic-class
branch
from
8c45531 to
29778a5
Compare
roboquat
added
release-note-none
and removed
do-not-merge/release-note-label-needed
labels
github-actions
bot
added
team: delivery
| const [workspaceClass, setWorkspaceClass] = useState<string>( | ||
| user?.additionalData?.workspaceClasses?.regular || "g1-standard", | ||
| ); | ||
| const [workspaceClass, setWorkspaceClass] = useState<string>(user?.additionalData?.workspaceClasses?.regular || ""); |
There was a problem hiding this comment.
nit: On first load I was confused because no class was selected.
It would make sense to highlight the one with isDefault: true, though. Maybe by moving the WorkspacClasses code into gitpod-protocol, und re-using it here...? 🤔 Or we introduce an explicit API method: getDefaultWorkspaceClass 🤷
There was a problem hiding this comment.
☝️ This can easily be a follow-up issue. 🧘
| const actuallySetWorkspaceClass = async (value: string) => { | ||
| const additionalData = user?.additionalData || {}; | ||
| const prevWorkspaceClass = additionalData?.workspaceClasses?.regular || "g1-standard"; | ||
| const prevWorkspaceClass = additionalData?.workspaceClasses?.regular || ""; |
There was a problem hiding this comment.
nit: The preview env classes read "Standard" and "Default", where Default is the bigger one which was somewhat confusing.
💡 We could re-use the SaaS names (+ maybe sizes?).
There was a problem hiding this comment.
Ah, that is only because I edited the configmap to better illustrate what it would look like. The installer only installs the Default workspace class as Tarun has posted below.
| description="Up to 8 vCPU, 16GB memory, 50GB disk" | ||
| powerUps={2} | ||
| /> | ||
| {supportedClasses.map((c) => { |
There was a problem hiding this comment.
Nice! Now it becomes apparent that this config is far to hidden. But now we have everything in place to maybe show it on /workspaces / the "open workspace modal" / somewhere else for visible? 😇
☁️ Clearly out-of-scope here, just wanted to raise in this context.
/cc @gtsiolis
There was a problem hiding this comment.
Awesome Work.
From a default install of self-hosted, I see the following
Shouldn't this class be selected by default? It could be confusing for users otherwise 🤔 Also, selecting it manually does not change anything as we would expect.
(@geropl also seems to have commented about the same. 👍🏼 )
Furisto
force-pushed
the
fo/dynamic-class
branch
from
cd35c29 to
15875d2
Compare
|
@Pothulapati Now the default class is selected if no class has been selected yet. I could not reproduce that selecting the workspace class does not do anything. |
Description
Source the workspace classes that should be displayed in the settings dialog from the server instead of hard coding them.
Related Issue(s)
Fixes ##11583
How to test
Release Notes
Werft options: