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.

Sign up for GitHub

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 Validation check to see if func is already initialized #2574

Merged
merged 2 commits into from
Nov 15, 2024
Merged

Add Validation check to see if func is already initialized #2574

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c2267b9
Add Validation check to see if func is already initialized
tarunsunny3 Nov 6, 2024
d9a9159
Refactor func init check in build.go
tarunsunny3 Nov 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Refactor func init check in build.go
  • Loading branch information
@tarunsunny3
tarunsunny3 committed Nov 15, 2024
commit d9a91595118bacc7eb05fd19945751e83287cb89
10 changes: 3 additions & 7 deletions cmd/build.go
View file
Original file line number Diff line number Diff line change
@@ -303,17 +303,13 @@ func (c buildConfig) Prompt() (buildConfig, error) {
// value and will always use the value from the config (flag or env variable).
// This is not strictly correct and will be fixed when Global Config: Function
// Context is available (PR#1416)
hasFunc, err := fn.IsFunctionInitialized(c.Path)
if err != nil {
return c, err
}
if !hasFunc {
return c, fmt.Errorf("no function has been initialized in the current directory. Please initialize a function by running either:\n- func init --language <your language>\n- func create <function name> --language <your_language>")
}
f, err := fn.NewFunction(c.Path)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should not this already return ENOENT?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could simply check f.Initialized() instead of introducing new IsFunctionInitialized() function.

Copy link
Contributor

Choose a reason for hiding this comment

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

TBH I am surprised that the function returns err==nil when there is no function. Why it does that @lkingland ?

func/pkg/functions/function.go

Lines 257 to 265 in e6fa020

// If no func.yaml in directory, return the default function which will
// have f.Initialized() == false
var filename = filepath.Join(root, FunctionFile)
if _, err = os.Stat(filename); err != nil {
if os.IsNotExist(err) {
err = nil
}
return
}

Copy link
Contributor

Choose a reason for hiding this comment

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

@lkingland also if you mean zero-value by "default function" , then you also need to zero the path of the f.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, good question. A Function is first a structure in memory, which can be serialized to disk. The path to where the Function is, or will be when initialized, is the only value provided in the constructor, and thus is the only field that is populated with user data when creating a new function from scratch.

Like you say. Just check if f.Initialized().

Copy link
Contributor

Choose a reason for hiding this comment

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

I still do not like, but can live with it. Just the function docstring says:

NewFunction from a given path. Invalid paths, or no function at path are errors.

Copy link
Contributor

Choose a reason for hiding this comment

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

I bet that if you asked 10 programmers what NewFunction(pathWithNoFunction) returns at least 9 would tell it's an error.

if err != nil {
return c, err
}
if !f.Initialized() {
return c, fmt.Errorf("no function has been initialized in %q. Please initialize a function by running:\n- func init --language <your language>", c.Path)
}
if (f.Registry == "" && c.Registry == "" && c.Image == "") || c.Confirm {
fmt.Println("A registry for function images is required. For example, 'docker.io/tigerteam'.")
err := survey.AskOne(
3 changes: 0 additions & 3 deletions pkg/functions/client.go
View file
Original file line number Diff line number Diff line change
@@ -1276,9 +1276,6 @@ func isEffectivelyEmpty(path string) (bool, error) {
}
return true, nil
}
func IsFunctionInitialized(path string) (bool, error) {
return hasInitializedFunction(path)
}

// returns true if the given path contains an initialized function.
func hasInitializedFunction(path string) (bool, error) {
Loading