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

Refactor github.com/kubernetes/helm/pkg/strvals away #926

Closed
mstoykov opened this issue Feb 19, 2019 · 3 comments
Closed

Refactor github.com/kubernetes/helm/pkg/strvals away #926

mstoykov opened this issue Feb 19, 2019 · 3 comments
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 maintenance refactor
Milestone

Comments

@mstoykov
Copy link
Contributor

No description provided.

@na-- na-- added the evaluation needed proposal needs to be validated or tested before fully implementing it in k6 label Jul 29, 2020
@na--
Copy link
Member

na-- commented Jul 29, 2020

Played with this some and came up with a simplistic but somewhat viable replacement: https://play.golang.org/p/6Y23Fyv8gO5

Copying it here, since I'm not sure if Go playground links are permanent:

package main

import (
	"fmt"
	"strconv"
	"strings"
)

func main() {
	dump(`test=mest,asdf=123, true=false`)
	dump(`we are pretty liberal = with what we support, even=" quoted strings with spaces  "`)
	dump(`noval,noval2=,noval3= , emptyval="",someval=wat`)
	dump(`test=mest,foo="bar",int=123,test=overwrite , float=3.14, sub.a=1, , sub.b=2, sub.c="Asdf"`)
	dump(`test=mest,test.sub=this is an error`)
}

func dump(s string) {
	res, err := strvals(s)
	fmt.Printf("err=%v, res=%#v\n", err, res)
}

func strvals(s string) (map[string]interface{}, error) {
	res := map[string]interface{}{}

	keyvals := strings.Split(s, ",")

	for _, kv := range keyvals {
		if len(kv) == 0 {
			continue
		}
		kvSplit := strings.SplitN(kv, "=", 2)
		if len(kvSplit) == 1 {
			kvSplit = append(kvSplit, "")
		}

		keyParts := strings.Split(strings.TrimSpace(kvSplit[0]), ".")
		resLevel := res

		for i := 0; i < len(keyParts)-1; i++ {
			if newLevel, ok := resLevel[keyParts[i]]; !ok {
				newLevel := map[string]interface{}{}
				resLevel[keyParts[i]] = newLevel
				resLevel = newLevel
			} else {
				if newLevelMap, ok := newLevel.(map[string]interface{}); ok {
					resLevel = newLevelMap
				} else {
					return nil, fmt.Errorf(
						"option '%s' was cannot be both %q and a map",
						strings.Join(keyParts[:i+1], "."), newLevel,
					)
				}
			}
		}

		resLevel[keyParts[len(keyParts)-1]] = getVal(strings.TrimSpace(kvSplit[1]))
	}

	return res, nil
}

func getVal(val string) interface{} {
	if val == "" {
		return nil
	} else if iVal, err := strconv.ParseInt(val, 10, 64); err == nil {
		return iVal
	} else if fVal, err := strconv.ParseFloat(val, 64); err == nil {
		return fVal
	} else if bVal, err := strconv.ParseBool(val); err == nil {
		return bVal
	} else if len(val) >= 2 && val[0] == '"' && val[len(val)-1] == '"' {
		return val[1 : len(val)-1]
	} else {
		return val
	}
}

running it will produce:

err=<nil>, res=map[string]interface {}{"asdf":123, "test":"mest", "true":false}
err=<nil>, res=map[string]interface {}{"even":" quoted strings with spaces  ", "we are pretty liberal":"with what we support"}
err=<nil>, res=map[string]interface {}{"emptyval":"", "noval":interface {}(nil), "noval2":interface {}(nil), "noval3":interface {}(nil), "someval":"wat"}
err=<nil>, res=map[string]interface {}{"":interface {}(nil), "float":3.14, "foo":"bar", "int":123, "sub":map[string]interface {}{"a":1, "b":2, "c":"Asdf"}, "test":"overwrite"}
err=option 'test' was cannot be both "mest" and a map, res=map[string]interface {}(nil)

@mstoykov
Copy link
Contributor Author

mstoykov commented Apr 6, 2021

This is a requirement to drop gopkg.in/yaml.v2 (we still will have the v3).

In addition, we need to drop it from some 2 places in our code base and drop github.com/zyedidia/highlight as it also uses the v2 and there are no updates that will replace it with something else.

This in total drops around 12k LOC.

On the other side the kafka output is making a lot of use of it to a point that redoing it is probably not ... possible. But the other places we use it (influxdb and --dns cli argument) are probably not using all the strvals functionality

@na--
Copy link
Member

na-- commented Nov 30, 2021

I'll close this issue because we no longer depend on strvals in k6 after #2270 🎉 That said, we have hand-crafted a bunch of poor replacements in the few places where we need something like it... It will be much simpler and less error prone to have that type of config parsing to be done by a config library... So I opened grafana/croconf#16, which we'll hopefully get around to again soon 🤞

@na-- na-- closed this as completed Nov 30, 2021
@na-- na-- added this to the v0.36.0 milestone Nov 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 maintenance refactor
Projects
None yet
Development

No branches or pull requests

2 participants