-
Notifications
You must be signed in to change notification settings - Fork 21
Replace Base64 encoding by Gzip compression #39
Conversation
45c339b
to
8ea7131
Compare
1 similar comment
kv.go
Outdated
@@ -155,11 +158,21 @@ func decodeHook(fromType reflect.Type, toType reflect.Type, data interface{}) (i | |||
|
|||
return dataOutput, nil | |||
} else if fromType.Kind() == reflect.String { | |||
b, err := base64.StdEncoding.DecodeString(data.(string)) | |||
buffer := bytes.NewBuffer([]byte(data.(string))) | |||
reader, err := gzip.NewReader(buffer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could make that a bit more composable (might be overkill for the current codebase though — and I'm really bad at naming functions)
func base64Reader(r io.Reader) (*Reader, error) {
return base64.NewDecoder(base64.StdEncoding, r)
}
func readByPrecedence(r io.Reader, fs ...func(io.Reader) (*Reader, error)) ([]byte, error) {
var err error
for _, f := range fs {
reader, err := f(r)
if err == nil {
return ioutil.ReadAll(reader)
}
}
return nil, err
}
// […]
data := bytes.NewBuffer([]byte(data.(string)))
reaturn readByPrecedence(data, gzip.NewReader, base64Reader)
// […]
kv.go
Outdated
} | ||
} | ||
return data, nil | ||
} | ||
|
||
func gzipReader(data interface{}) (interface{}, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interface{}
in arg and return seems a bit risky if you wanna reuse that func. I would do the cast before and return io.Reader, error
👼
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
kv.go
Outdated
} | ||
|
||
func base64Reader(data string) ([]byte, error) { | ||
return base64.StdEncoding.DecodeString(data) | ||
func gzipReader(r io.Reader) (io.Reader, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need that, just pass gzip.NewReader
when calling readCompressedData
😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, the func signature differ
LGTM 🎉
Today, byte array are Base64 encoded before to be stored in a KV store.
But the size of these data can be a problem.
Indeed, KV stores, like Consul, can only store values with a size up to 500 KB.
This PR allows replacing Base64 encoded data by gzip compressed data to reduce the size of these data.
Note : If data cannot be unzipped, they are Base64 decoded to preserve the backward compatibility.
Unit tests have been updated too.
Related to traefik/traefik#1325