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

Add persistent volume claim to prebuild settings #10539

Merged
merged 1 commit into from
Jun 14, 2022
Merged

Conversation

sagor999
Copy link
Contributor

@sagor999 sagor999 commented Jun 8, 2022

Description

This adds setting to prebuilds to enable support for persistent volume claims.
Made sure to add a warning to let users know to not enable this setting, but I am open for a better alternative\text or if there is way to gate this feature to gitpod only for now.

This is how it would look:
Screenshot from 2022-06-08 16-41-29

Related Issue(s)

Related: #10260

How to test

Launch preview env, add your project to it and navigate to settings.

Release Notes

none

Documentation

@sagor999 sagor999 changed the title wip Add persistent volume claim to prebuild settings Jun 8, 2022
@sagor999 sagor999 marked this pull request as ready for review June 8, 2022 23:44
@sagor999 sagor999 requested a review from a team June 8, 2022 23:44
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Jun 8, 2022
@easyCZ
Copy link
Member

easyCZ commented Jun 9, 2022

Hey @sagor999, this is a great use-case for FeatureFlags we've recently introduced. We've not got too much in the way of write up for this so I'm happy to pair initially to bring you up to speed. I'll then use this as the basis for some write-up to make it a bit more self-serve. Let me know when you're free or drop something in my calendar.

You can also have a look at existing usage in

https://github.com/gitpod-io/gitpod/blob/main/components/dashboard/src/Menu.tsx#L164-L182

@sagor999 sagor999 marked this pull request as draft June 9, 2022 17:14
@sagor999 sagor999 marked this pull request as ready for review June 9, 2022 23:26
@sagor999
Copy link
Contributor Author

sagor999 commented Jun 9, 2022

@easyCZ PTAL

Copy link
Member

@easyCZ easyCZ left a comment

Choose a reason for hiding this comment

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

Usage of the feature flag looks good to me, and in general PR looks good. I'm no expert in React however. Adding hold in case you want to get a review from someone else as well.

/hold

@sagor999
Copy link
Contributor Author

@jankeromnes would you be able to take a look at React portion?

@geropl
Copy link
Member

geropl commented Jun 13, 2022

@sagor999 Did you think about replacing that project-level config with a feature-flag entirely? 🤔 Or is it that you really want to expose this option to users? 🤔

@geropl geropl self-assigned this Jun 13, 2022
@sagor999
Copy link
Contributor Author

@geropl yes, I would like it being exposed to users, as we will slowly start offering this to some of our SaaS customers to try it out, if we think it will be a great benefit to them and they are willing to test it out.
I also want to be able to switch it on and off for a project without having to do it in configcat.

Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

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

Code LGTM, and works!

@sagor999
Copy link
Contributor Author

/unhold

@roboquat roboquat merged commit 28f48ac into main Jun 14, 2022
@roboquat roboquat deleted the pavel/prebuilds branch June 14, 2022 14:40
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note-none size/M team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants