Skip to content

tiltfile: add yaml function#1111

Merged
jazzdan merged 4 commits into
masterfrom
dmiller/ch1459/add-blob-function
Feb 5, 2019
Merged

tiltfile: add yaml function#1111
jazzdan merged 4 commits into
masterfrom
dmiller/ch1459/add-blob-function

Conversation

@jazzdan

@jazzdan jazzdan commented Feb 4, 2019

Copy link
Copy Markdown
Contributor

So that you can take a Starlark string and pass it in to k8s_yaml, as demonstrated in the unit test.

This is the last part of #894.

@jazzdan
jazzdan requested review from dbentley and nicks February 4, 2019 23:08
Comment thread docs/api/api.py Outdated
def fail(msg: str) -> None:
"""Raises an error that cannot be intercepted. Can be used anywhere in a Tiltfile."""

def blob(input: str) -> Blob:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i'm still concerned that blob is a bad name for this function, but not sure i can come up with a better one

Comment thread internal/tiltfile/files.go Outdated

func (s *tiltfileState) blob(thread *starlark.Thread, fn *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
var input starlark.String
err := starlark.UnpackArgs(fn.Name(), args, kwargs, "path", &input)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"path" -> "input" ?

Comment thread internal/tiltfile/files.go Outdated
var input starlark.String
err := starlark.UnpackArgs(fn.Name(), args, kwargs, "path", &input)
if err != nil {
return nil, fmt.Errorf("Argument 0 (path): %v", err)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i don't think you need this fmt.Error? UnpackArgs should already give you a far better and more descriptive message.

@nicks

nicks commented Feb 5, 2019

Copy link
Copy Markdown
Contributor

i guess the Go way to do this would be to have k8s_yaml accept a YAML type, and make the conversion from blob -> yaml implicit. So the function would be yaml('string')? but if you want to go with blob, that's ok too

@jazzdan

jazzdan commented Feb 5, 2019

Copy link
Copy Markdown
Contributor Author

@nicks I like that approach better, I'm going to do that.

@jazzdan
jazzdan force-pushed the dmiller/ch1459/add-blob-function branch from d804f4c to 21b7918 Compare February 5, 2019 20:54
@jazzdan jazzdan changed the title tiltfile: add blob function tiltfile: add yaml function Feb 5, 2019
@jazzdan
jazzdan merged commit 1471a9f into master Feb 5, 2019
@nicks
nicks deleted the dmiller/ch1459/add-blob-function branch March 31, 2021 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants