feat: decouple function name from function domain #127
Conversation
|
|
||
| // Create project root directory, if it doesn't already exist | ||
| if err = os.MkdirAll(cfg.Root, 0755); err != nil { | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
I really like the idea of creating the directory in which a function will be deployed because it would ensure the function is in a clean fresh directory. However, one small question: what do we do when a user wants to start over, without also removing their git history (.git) or other ancillary config files (.direnv)? This is why the code exists to check if a directory is effectively empty: contains no visible files, and no .faas.yaml, before allowing a re-run of create or init.
There was a problem hiding this comment.
If I am not mistaken, this command is not doing anything if the directory already exists (and contains some hidden files).
In that case user would do mkdir my_dir_with_hidden_files && faas init my_dir_with_hidden_files
| Path: prompt.ForString("Project path", c.initConfig.Path), | ||
| Name: prompt.ForString("Project name", name, prompt.WithRequired(true)), | ||
| Name: derivedName, | ||
| Path: derivedPath, |
There was a problem hiding this comment.
It appears to me that perhaps this omits the prompting of name and path entirely, right?
I was actuall fond of the original idea: to prompt if the parameter were not provided as a flag. If that's not the plan anymore, no problem! But if we wanted to prompt the user when the parameters had not been provided, would we not instead leave things roughly as they were, and insteaed add here a conditional that only prompts if the flag does not exist (as provided by the CLI API)?
There was a problem hiding this comment.
It appears to me that perhaps this omits the prompting of name and path entirely, right?
@lkingland I may be misreading your comment, so disregard if so (or correct me). The prompt occurs on line 121.
derivedName, derivedPath := deriveNameAndAbsolutePathFromPath(prompt.ForString("Project path", c.initConfig.Path, prompt.WithRequired(true)))
|
@lkingland thanks for all the comments! This is still a draft and I was about to refactor/rewrite some parts of it, based on the conversation on #125 and Slack etc. |
lance
changed the title
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
zroubalik
changed the title
|
@boson-project/core PTAL |
| Path: prompt.ForString("Project path", c.initConfig.Path), | ||
| Name: prompt.ForString("Project name", name, prompt.WithRequired(true)), | ||
| Name: derivedName, | ||
| Path: derivedPath, |
There was a problem hiding this comment.
It appears to me that perhaps this omits the prompting of name and path entirely, right?
@lkingland I may be misreading your comment, so disregard if so (or correct me). The prompt occurs on line 121.
derivedName, derivedPath := deriveNameAndAbsolutePathFromPath(prompt.ForString("Project path", c.initConfig.Path, prompt.WithRequired(true)))| @@ -165,21 +166,42 @@ func functionWithOverrides(root, namespace, image string) (f faas.Function, err | |||
|
|
|||
| // deriveName returns the explicit value (if provided) or attempts to derive | |||
| // from the given path. Path is defaulted to current working directory, where | |||
| // a function configuration, if it exists and contains a name, is used. Lastly | |||
| // derivation using the path us used. | |||
| // a Function configuration, if it exists and contains a name, is used. | |||
| func deriveName(explicitName string, path string) string { | |||
There was a problem hiding this comment.
I wonder if this function could be better named as it no longer really derives the name, but instead just pulls it from the configuration file or returns the passed value.
There was a problem hiding this comment.
Any suggestion?
I've been sitting here for 5 minutes trying to come up with a good name. I see why you asked. The best I've come up with is providedOrConfiguredName(provided, path string) string. But meh.
There was a problem hiding this comment.
Hehe, yeah 😄 I don't like the name either, so I am happy to change it to anything else, that makes sense for everyone
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>
Signed-off-by: Zbynek Roubalik zroubali@redhat.com
Fixes: #123
Creates a new directory foo and initialize/create the function inside.
A directory structure is created according to the specified name, eg.
relative/path/to/foo,/absolute/path/to/foo,fooor empty value (in this case it uses current directory).The 'last' directory name is used as function name.