Add hybrid mode content #450
Conversation
omnomagonz
requested review from
RinkiyaKeDad,
rberrelleza and
pchico83
as code owners
✅ Deploy Preview for okteto-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
@pchico83 @rberrelleza @RinkiyaKeDad please review when you have a moment 🙏 |
| workdir: ./frontend | ||
| command: ["npx webpack watch --mode development"] | ||
| ``` | ||
|
|
There was a problem hiding this comment.
With the new hybrid not all parameters in the dev section apply in this mode. Shouldn't we add a clarification in every one of these parameters stating if it's supported in sync and/or hybrid mode?
Example:
### envFiles
> Supported in both `sync` and `hybrid` modes.
...
### affinity
> Only supported in `sync` mode.
...
There was a problem hiding this comment.
@rlamana Ah yes. I think that's a good and important clarification to add.
Are those the only two exceptions or do we need to compile a complete list?
There was a problem hiding this comment.
No, we have to take into account all of them. The list is compiled in the spec doc here: https://www.notion.so/okteto/Okteto-CLI-3-0-7fbd3a6a566145d3a4930d3c15fc642d?pvs=4#37aeeb1c05824203ac5f8132caf76e1f
And also the work done here: okteto/okteto#3808
@andreafalzetti @teresaromero you might have more context if that table there is updated after the work done there or is there anything we need to change?
There was a problem hiding this comment.
@omnomagonz @rlamana the following might require a bit of polishing but should include almost everything:
These are the fields that are supported in sync but not in hybrid:
- affinity
- context
- externalVolumes
- image
- imagePullPolicy
- initContainer
- initFromImage
- lifecycle
- namespace
- nodeSelector
- persistentVolume
- push
- replicas
- secrets
- securityContext
- serviceAccount
- sshServerPort
- sync
- tolerations
- volumes
These fields work in both modes:
- annotations
- autocreate
- command
- container
- envFiles
- environment
- environment
- forward
- metadata
- mode
- reverse
- selector
- workdir
These fields work in sync but we're not sure if they make sense for hybrid:
- interface
- probes
- remote
- timeout
This field exists in code but not documented and not sure if it should be supported:
- args
Should work in both but deprecated:
- healthchecks
- labels
There was a problem hiding this comment.
Thanks a lot @andreafalzetti for the updated list. We will need to clarify those that we are not sure:
- interface
- probes
- remote
- timeout
As discussed offline maybe @pchico83 can help us with more context on these, and soon before he leaves on PTO.
There was a problem hiding this comment.
Resolving this conversation; please see latest commit: d45fc31 and add additional comments :)
I left out some of the nuanced details like "deprecated but works in both" and "exists in code but not documented". We can iterate to add more clarity on these but I didn't want to get blocked on nuance for now
There was a problem hiding this comment.
@omnomagonz "not sure if they make sense for hybrid" meant that we are still deciding if the will fall under the "ignored/unsupported" fields or if we will implement it in both modes. But there will be a final resolution for all those so that the final list will only have "supported or not supported" for each mode.
@andreafalzetti what is the current status of those fields?
There was a problem hiding this comment.
In my opinion, besides adding a table I will still add a "hint" on every single parameter. It is common to just go and check a parameter in the docs to learn about it. If the user doesn't have that info there (and assuming they are aware of these two modes and know there is a table for it), users will have to go to look for the table and see if the parameter is there, and then maybe go back to the parameter description to continue reading about it. I find the table less important since you usually go seeking for documentation on a particular item. It is pretty common to see this type of support descriptions in docs of this kind.
There was a problem hiding this comment.
@rlamana for remote I was able to verify that it works for hybrid too. timeout for me doesn't work in sync either, so I've created okteto/okteto#3849
these two work:
- interface
- probes
There was a problem hiding this comment.
@andreafalzetti So after our daily today, I think probes was clarified and it's good to go. What about interface then?
| - `sync`: (default) Sync & run your code in a remote development container. | ||
| - `hybrid`: Run your code on your local computer instead of in a remote development container, while rest of your service components runs in your remote cluster. | ||
|
|
||
| | | `sync` (default) | `hybrid` | |
There was a problem hiding this comment.
I think we have the same table in cloud/okteto-cli. Should we link that table here instead of repeating it?
There was a problem hiding this comment.
My vote would be to repeat it because I think it's important in both sections and is succinct.
If the content was more verbose then I'd say link to it, but since this is fairly concise I think it's worth saving the user a trip to another doc page so they can stay in the same context.
Happy to hear contrary opinion though 😄
src/content/cloud/okteto-cli.mdx
Outdated
|
|
||
| Below is a reference to help you configure your [`dev` section](/reference/manifest/#dev-object-optional) based on the mode you select. **Standard** configuration options work in both modes and **Sync** options work *only* when `sync` is selected. | ||
|
|
||
| For example, if you select `hyrid` mode, you can only use the configuration options in your manifest `dev` section listed under **Standard**. Any configuration options in the **Sync** list will *not work*. |
There was a problem hiding this comment.
IMO it seems strange to list under standard the hybrid mode (considering that it is not the default mode). Shouldn't we refer to the modes here and in the section below as hybrid or sync under this context to avoid confusion?
There was a problem hiding this comment.
I would avoid saying things like "will not work", I'd say "some configuration options do not apply an will be ignore", or something along those line.
The thing is, all that sounds like we don't want to support it or like if it is unfinished, but the reality is that some options don't make sense given the nature of the selected "mode".
On the other hand, there is a use case that a user might want to switch from one mode to another (maybe testing what mode works best for them or just because it is better suited for an specific case). In that case, they don't need to remove all those options that don't apply in their manifest, those not supported will just be ignored.
Hope this help to explain better the nuances of both modes.
There was a problem hiding this comment.
@omnomagonz I also don't get what exactly "Standard" means?
There was a problem hiding this comment.
The reason I landed on Standard vs. Sync (which I agree doesn't sound right in hindsight) was because it didn't make sense to list: Standard (works for both), sync, and hybrid (duplicate of what's listed in Standard).
One option is to list Sync vs. Hybrid and just list all compatible commands even if it means duplicating some.
And I'll make some adjustments to language to avoid "will not work" 👍
Co-authored-by: adripedriza <adripedriza@gmail.com>
Co-authored-by: Ramón Lamana <rlamana@gmail.com>
|
Closing this PR for now |
Based on #447 I'm proposing we add the
hybridmode content in these areas and using this language.Opening a separate PR seemed cleaner 😄