jason810496 commented on code in PR #75602:
URL: https://github.com/apache/airflow/pull/75602#discussion_r4548152311
##########
airflow-core/docs/authoring-and-scheduling/language-sdks/go.rst:
##########
@@ -494,6 +495,53 @@ retries it if the task has retries.
)
require.NoError(t, transform(actx, "uk", map[string]any{"rows": 42}))
+.. _go-sdk/dag-run-conf:
+
+Reading the Dag run conf
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+Reading ``Conf`` directly gives the value of a key as an ``any``, so a task
that reads several keys asserts a Go
+type per key. ``UnmarshalConf`` reads the conf into a struct with typed fields
instead:
+
+.. code-block:: go
+
+ type etlConf struct {
+ TargetTable string
+ Limit int
+ Since time.Time
+ }
+
+ func load(actx airflow.Context) error {
+ etl := etlConf{Limit: 100}
+ if err := actx.DagRun().UnmarshalConf(&etl); err != nil {
+ return err
+ }
+ ...
+ }
+
+For a run triggered with the conf ``{"target_table": "sales", "since":
"2024-01-31T00:00:00Z"}``,
+``etl.TargetTable`` is ``"sales"``, ``etl.Since`` is that date, and
``etl.Limit`` stays ``100``.
+
+* Each key goes to the field whose name matches it when case and underscores
are ignored, so ``target_table``
+ reaches ``TargetTable``. A ``json:`` tag renames the field, and the tag name
matches the same way. Objects
+ nested in the conf follow the same rule, including objects in slices and
objects that are map values. The
+ keys of a map field stay as written, because they are data rather than field
names.
+* The ``arg:`` tag that binds a TaskFlow argument has no effect here. Read a
key that no field name can match,
+ such as ``target-table``, from ``Conf``, or name it in a ``json:`` tag.
+* A number decoded into an ``any``, such as a field of type ``any`` or a value
of a ``map[string]any`` or
+ ``[]any`` field, arrives as a ``json.Number`` rather than as the number type
``Conf`` carries, because a
+ ``float64`` cannot hold every integer a conf can. Read it with its ``Int64``
or ``Float64`` method, or give
+ the field a Go integer or float type to have the decoder convert it. Of
those, only an integer type keeps
+ every digit of a large integer.
+* Keys that match no field are ignored, because other tasks in the run may
read them. A field with no matching
+ key keeps the value the struct already holds, so set the defaults before the
call, as ``Limit`` does above.
+* The conf holds only the values the run was triggered with. In a Python task,
``params`` combines the conf
+ with the defaults of the Dag's params. ``UnmarshalConf`` does not add those
defaults, so in a scheduled run
+ every field keeps the value it came in with.
+* It returns an error when a value does not fit the type of its field, when
two keys such as ``target_table``
+ and ``TARGET_TABLE`` match one field and neither names it exactly, or when
two fields of the struct differ
+ only in case or underscores.
Review Comment:
This repeats the `UnmarshalConf` godoc and has drifted. Let's keep the
example and link the godoc.
```suggestion
For how keys match fields and when it returns an error,
see `DagRun.UnmarshalConf
<https://pkg.go.dev/github.com/apache/airflow/go-sdk/sdk#DagRun.UnmarshalConf>`__.
```
##########
go-sdk/sdk/conf.go:
##########
@@ -0,0 +1,370 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package sdk
+
+import (
+ "bytes"
+ "encoding"
+ "encoding/json"
+ "fmt"
+ "maps"
+ "reflect"
+ "slices"
+ "strings"
+ "sync"
+)
+
+// UnmarshalConf reads [DagRun.Conf] into target, which must be a non-nil
+// pointer, usually to a struct with one field per conf key:
+//
+// type etlConf struct {
+// TargetTable string
+// Limit int
+// Since time.Time
+// }
+//
+// etl := etlConf{Limit: 100}
+// if err := actx.DagRun().UnmarshalConf(&etl); err != nil {
+// return err
+// }
+//
+// For a run triggered with {"target_table": "sales", "since":
"2024-01-31T00:00:00Z"},
+// etl.TargetTable is "sales", etl.Since is that date, and etl.Limit stays 100.
+//
+// Each key goes to the field whose name matches it when case and underscores
+// are ignored, so target_table reaches TargetTable. A `json:` tag renames the
+// field, and the tag name matches the same way. If one key matches a field
+// name exactly, that key wins over keys that match it only once case and
+// underscores are ignored. Objects nested in the conf follow the same rules,
+// including objects in slices and objects that are map values. The keys of a
+// map field stay as written, because they are data rather than field names.
+//
+// Keys that match no field are ignored, because other tasks in the run may
+// read them. A field with no matching key keeps the value target already
+// holds, so set the defaults on target before the call, as Limit does above.
+// The conf carries only the values the run was triggered with: it does not
+// carry the defaults of the Dag's params, so in a scheduled run every field
+// keeps the value it came in with.
+//
+// The `arg:` tag that binds a TaskFlow argument has no effect here. Read a key
+// that no field name can match, such as target-table, from [DagRun.Conf], or
+// name it in a `json:` tag.
+//
+// A number decoded into an any, such as a field of type any or a value of a
+// map[string]any or []any field, arrives as a [json.Number] rather than as the
+// number type [DagRun.Conf] carries, because a float64 cannot hold every
+// integer a conf can. Read it with its Int64 or Float64 method, or give the
+// field a Go integer or float type to have the decoder convert it. Of those,
+// only an integer type keeps every digit of a large integer.
+//
+// It returns an error when a value does not fit the type of its field, when
+// two keys match one field only once case and underscores are ignored, or when
+// two fields of a struct differ only in case or underscores.
+func (d DagRun) UnmarshalConf(target any) error {
+ rv := reflect.ValueOf(target)
+ if rv.Kind() != reflect.Pointer || rv.IsNil() {
+ return fmt.Errorf(
+ "sdk.DagRun.UnmarshalConf: target must be a non-nil
pointer, got %T",
+ target,
+ )
+ }
+ matched, err := matchConfValue(d.Conf, rv.Type().Elem())
+ if err != nil {
+ return err
+ }
+ blob, err := json.Marshal(matched)
+ if err != nil {
+ return fmt.Errorf("sdk.DagRun.UnmarshalConf: cannot encode the
Dag run conf: %w", err)
+ }
+ dec := json.NewDecoder(bytes.NewReader(blob))
+ // A number that reaches an any-typed destination keeps its literal, so
the
+ // task reads an exact integer instead of the float64 encoding/json
would
+ // otherwise decode it into.
+ dec.UseNumber()
+ if err := dec.Decode(target); err != nil {
+ return fmt.Errorf(
+ "sdk.DagRun.UnmarshalConf: cannot read the Dag run conf
into %s: %w",
+ rv.Type().Elem(),
+ err,
+ )
+ }
+ return nil
+}
+
+// matchConfValue renames the keys of every conf object that becomes a struct
+// when the conf is decoded into target. encoding/json matches a key to a field
+// only when the two differ at most in case, so renaming the keys to the exact
+// field names is what lets target_table reach TargetTable.
+//
+// The walk follows target rather than the conf, so it reaches objects nested
+// in slices, arrays and map values, and stops at a type that decodes itself.
+func matchConfValue(value any, target reflect.Type) (any, error) {
+ for target.Kind() == reflect.Pointer {
+ target = target.Elem()
+ }
+ // A type such as time.Time decodes from a value of its own shape, and
its
+ // Go fields are not conf keys.
+ if implementsConfUnmarshaler(target) {
+ return value, nil
+ }
+
+ switch target.Kind() {
+ case reflect.Struct:
+ if obj, ok := value.(map[string]any); ok {
+ return matchConfFields(obj, target)
+ }
+ case reflect.Map:
+ if obj, ok := value.(map[string]any); ok {
+ matched := make(map[string]any, len(obj))
+ for key, elem := range obj {
+ elem, err := matchConfValue(elem, target.Elem())
+ if err != nil {
+ return nil, err
+ }
+ matched[key] = elem
+ }
+ return matched, nil
+ }
+ case reflect.Slice, reflect.Array:
+ if list, ok := value.([]any); ok {
+ matched := make([]any, len(list))
+ for i, elem := range list {
+ elem, err := matchConfValue(elem, target.Elem())
+ if err != nil {
+ return nil, err
+ }
+ matched[i] = elem
+ }
+ return matched, nil
+ }
+ }
+ // A value whose shape does not fit the field, such as a string for a
+ // struct, passes through so that the decoder reports the mismatch.
+ return value, nil
+}
+
+// matchConfFields renames each key of obj to the name of the target field the
+// key matches, and drops a key that matches no field. Keys are visited in
+// sorted order so that an error names the same pair of keys on every run.
+func matchConfFields(obj map[string]any, target reflect.Type) (map[string]any,
error) {
+ fields, err := confFieldsOf(target)
+ if err != nil {
+ return nil, err
+ }
+
+ matched := make(map[string]any, len(obj))
+ keyOfField := make(map[string]string, len(obj))
+ for _, key := range slices.Sorted(maps.Keys(obj)) {
+ field, ok := fields.byName[key]
+ if !ok {
+ // A key that folds to a field claims that field only
when no other
+ // key names it exactly.
+ folded, found := fields.byFold[foldConfName(key)]
+ if !found {
+ continue
+ }
+ if _, exact := obj[folded.name]; exact {
+ continue
+ }
+ field = folded
+ }
+ if other, dup := keyOfField[field.name]; dup {
+ return nil, fmt.Errorf(
+ "sdk.DagRun.UnmarshalConf: Dag run conf keys %q
and %q both match field %s when case and "+
+ "underscores are ignored, so the SDK
cannot tell which one to use; name the field or "+
+ "its json tag exactly as the key it
should take",
+ other,
+ key,
+ field.goName,
+ )
+ }
+ keyOfField[field.name] = key
+
+ value, err := matchConfValue(obj[key], field.typ)
+ if err != nil {
+ return nil, err
+ }
+ matched[field.name] = value
+ }
+ return matched, nil
+}
+
+// confField is one field a conf key can reach.
+type confField struct {
+ // name is the key the decoder matches the field by: the `json:` tag
name
+ // when the field has one, and the Go field name otherwise.
+ name string
+ // goName is the field as an error names it, qualified with the struct
that
+ // declares it, which is an embedded struct for a promoted field.
+ goName string
+ typ reflect.Type
+}
+
+type confFields struct {
+ byName map[string]confField
+ byFold map[string]confField
+ err error
+}
+
+// Indexing a struct costs far more than renaming the keys of one object of
+// that struct, so a conf holding a list of many objects would otherwise spend
+// most of its time indexing the same struct again.
+var confFieldCache sync.Map // reflect.Type -> *confFields
+
+func confFieldsOf(target reflect.Type) (*confFields, error) {
+ if cached, ok := confFieldCache.Load(target); ok {
+ fields := cached.(*confFields)
+ return fields, fields.err
+ }
+ fields := indexConfFields(target)
+ confFieldCache.Store(target, fields)
+ return fields, fields.err
+}
+
+func indexConfFields(target reflect.Type) *confFields {
+ fields := &confFields{
+ byName: map[string]confField{},
+ byFold: map[string]confField{},
+ }
+ // Breadth-first over embedded structs, because a field of the outer
struct
+ // shadows the field of the same name it promotes, as encoding/json
does.
+ // A struct can embed a pointer to itself, so each one is visited once.
+ visited := map[reflect.Type]bool{target: true}
+ for level := []reflect.Type{target}; len(level) > 0; {
+ var embedded []reflect.Type
+ for _, structType := range level {
+ for _, inner := range collectConfFields(structType,
fields) {
+ if !visited[inner] {
+ visited[inner] = true
+ embedded = append(embedded, inner)
+ }
+ }
+ }
+ if fields.err != nil {
+ return fields
+ }
+ level = embedded
+ }
+ return fields
+}
+
+// collectConfFields adds the fields one struct declares and returns the
+// structs it embeds, whose fields belong to the next level.
+func collectConfFields(structType reflect.Type, fields *confFields)
[]reflect.Type {
+ var embedded []reflect.Type
+ for i := range structType.NumField() {
+ f := structType.Field(i)
+ name, tagged := confFieldName(f)
+ // `json:"-"` on an embedded struct drops the fields it would
promote,
+ // as encoding/json does, so the check comes before the walk
into it.
+ // Promoting them instead can fail a struct the decoder fills,
with an
+ // error naming a field no conf key can reach.
+ if name == "" {
+ continue
+ }
+ if f.Anonymous && !tagged {
+ if inner := confStructType(f.Type); inner != nil {
+ embedded = append(embedded, inner)
+ continue
+ }
+ }
+ if !f.IsExported() {
+ continue
+ }
+ if _, shadowed := fields.byName[name]; shadowed {
+ continue
+ }
+ field := confField{
+ name: name,
+ goName: confTypeName(structType) + "." + f.Name,
+ typ: f.Type,
+ }
+ fields.byName[name] = field
+ folded := foldConfName(name)
+ if clash, dup := fields.byFold[folded]; dup {
+ fields.err = fmt.Errorf(
+ "sdk.DagRun.UnmarshalConf: fields %s and %s
differ only in case or underscores, so the SDK "+
+ "cannot tell which one a Dag run conf
key should go to; rename one of them",
+ clash.goName,
+ field.goName,
+ )
+ return nil
+ }
+ fields.byFold[folded] = field
+ }
+ return embedded
+}
+
+// confFieldName returns the key the decoder matches a field by, and whether a
+// `json:` tag names it. An empty name means the field takes no key at all,
+// which is what `json:"-"` asks for.
+func confFieldName(f reflect.StructField) (name string, tagged bool) {
+ tag, ok := f.Tag.Lookup("json")
+ if !ok {
+ return f.Name, false
+ }
+ tagName, _, _ := strings.Cut(tag, ",")
+ switch tagName {
+ case "-":
+ // `json:"-,"` names the field "-"; only a bare `json:"-"`
drops it.
+ if tag == "-" {
+ return "", false
+ }
+ return "-", true
+ case "":
+ return f.Name, false
+ }
+ return tagName, true
+}
+
+// confStructType returns the struct an embedded field promotes fields from, or
+// nil when the field promotes no conf key.
+func confStructType(t reflect.Type) reflect.Type {
+ if t.Kind() == reflect.Pointer {
+ t = t.Elem()
+ }
+ if t.Kind() != reflect.Struct || implementsConfUnmarshaler(t) {
+ return nil
+ }
+ return t
+}
+
+func foldConfName(name string) string {
+ return strings.ToLower(strings.ReplaceAll(name, "_", ""))
+}
+
Review Comment:
This duplicates `binding.foldArgName`. Since `binding` imports `sdk`, move
it to an `internal` package:
```go
// go-sdk/internal/fieldname/fieldname.go
package fieldname
import "strings"
// Fold lowercases name and drops underscores, so target_table matches
TargetTable.
func Fold(name string) string {
return strings.ToLower(strings.ReplaceAll(name, "_", ""))
}
```
```suggestion
```
##########
go-sdk/sdk/conf.go:
##########
@@ -0,0 +1,370 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package sdk
+
+import (
+ "bytes"
+ "encoding"
+ "encoding/json"
+ "fmt"
+ "maps"
+ "reflect"
+ "slices"
+ "strings"
+ "sync"
+)
+
+// UnmarshalConf reads [DagRun.Conf] into target, which must be a non-nil
+// pointer, usually to a struct with one field per conf key:
+//
+// type etlConf struct {
+// TargetTable string
+// Limit int
+// Since time.Time
+// }
+//
+// etl := etlConf{Limit: 100}
+// if err := actx.DagRun().UnmarshalConf(&etl); err != nil {
+// return err
+// }
+//
+// For a run triggered with {"target_table": "sales", "since":
"2024-01-31T00:00:00Z"},
+// etl.TargetTable is "sales", etl.Since is that date, and etl.Limit stays 100.
+//
+// Each key goes to the field whose name matches it when case and underscores
+// are ignored, so target_table reaches TargetTable. A `json:` tag renames the
+// field, and the tag name matches the same way. If one key matches a field
+// name exactly, that key wins over keys that match it only once case and
+// underscores are ignored. Objects nested in the conf follow the same rules,
+// including objects in slices and objects that are map values. The keys of a
+// map field stay as written, because they are data rather than field names.
+//
+// Keys that match no field are ignored, because other tasks in the run may
+// read them. A field with no matching key keeps the value target already
+// holds, so set the defaults on target before the call, as Limit does above.
+// The conf carries only the values the run was triggered with: it does not
+// carry the defaults of the Dag's params, so in a scheduled run every field
+// keeps the value it came in with.
+//
+// The `arg:` tag that binds a TaskFlow argument has no effect here. Read a key
+// that no field name can match, such as target-table, from [DagRun.Conf], or
+// name it in a `json:` tag.
+//
+// A number decoded into an any, such as a field of type any or a value of a
+// map[string]any or []any field, arrives as a [json.Number] rather than as the
+// number type [DagRun.Conf] carries, because a float64 cannot hold every
+// integer a conf can. Read it with its Int64 or Float64 method, or give the
+// field a Go integer or float type to have the decoder convert it. Of those,
+// only an integer type keeps every digit of a large integer.
+//
+// It returns an error when a value does not fit the type of its field, when
+// two keys match one field only once case and underscores are ignored, or when
+// two fields of a struct differ only in case or underscores.
+func (d DagRun) UnmarshalConf(target any) error {
+ rv := reflect.ValueOf(target)
+ if rv.Kind() != reflect.Pointer || rv.IsNil() {
+ return fmt.Errorf(
+ "sdk.DagRun.UnmarshalConf: target must be a non-nil
pointer, got %T",
+ target,
+ )
+ }
+ matched, err := matchConfValue(d.Conf, rv.Type().Elem())
+ if err != nil {
+ return err
+ }
Review Comment:
With a nil `Conf`, a nil map target comes back as an empty map.
```suggestion
matched, err := matchConfValue(d.Conf, rv.Type().Elem())
if err != nil {
return err
}
if d.Conf == nil {
return nil
}
```
##########
go-sdk/sdk/conf.go:
##########
@@ -0,0 +1,370 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package sdk
+
+import (
+ "bytes"
+ "encoding"
+ "encoding/json"
+ "fmt"
+ "maps"
+ "reflect"
+ "slices"
+ "strings"
+ "sync"
+)
+
+// UnmarshalConf reads [DagRun.Conf] into target, which must be a non-nil
+// pointer, usually to a struct with one field per conf key:
+//
+// type etlConf struct {
+// TargetTable string
+// Limit int
+// Since time.Time
+// }
+//
+// etl := etlConf{Limit: 100}
+// if err := actx.DagRun().UnmarshalConf(&etl); err != nil {
+// return err
+// }
+//
+// For a run triggered with {"target_table": "sales", "since":
"2024-01-31T00:00:00Z"},
+// etl.TargetTable is "sales", etl.Since is that date, and etl.Limit stays 100.
+//
+// Each key goes to the field whose name matches it when case and underscores
+// are ignored, so target_table reaches TargetTable. A `json:` tag renames the
+// field, and the tag name matches the same way. If one key matches a field
+// name exactly, that key wins over keys that match it only once case and
+// underscores are ignored. Objects nested in the conf follow the same rules,
+// including objects in slices and objects that are map values. The keys of a
+// map field stay as written, because they are data rather than field names.
Review Comment:
Please mention that other tag options still apply: `,string` rejects a
numeric conf value.
```suggestion
// map field stay as written, because they are data rather than field names.
// Other `json:` tag options keep their encoding/json meaning, so a `,string`
// field needs its conf value as a quoted string.
```
##########
go-sdk/sdk/conf.go:
##########
@@ -0,0 +1,370 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package sdk
+
+import (
+ "bytes"
+ "encoding"
+ "encoding/json"
+ "fmt"
+ "maps"
+ "reflect"
+ "slices"
+ "strings"
+ "sync"
+)
+
+// UnmarshalConf reads [DagRun.Conf] into target, which must be a non-nil
+// pointer, usually to a struct with one field per conf key:
+//
+// type etlConf struct {
+// TargetTable string
+// Limit int
+// Since time.Time
+// }
+//
+// etl := etlConf{Limit: 100}
+// if err := actx.DagRun().UnmarshalConf(&etl); err != nil {
+// return err
+// }
+//
+// For a run triggered with {"target_table": "sales", "since":
"2024-01-31T00:00:00Z"},
+// etl.TargetTable is "sales", etl.Since is that date, and etl.Limit stays 100.
+//
+// Each key goes to the field whose name matches it when case and underscores
+// are ignored, so target_table reaches TargetTable. A `json:` tag renames the
+// field, and the tag name matches the same way. If one key matches a field
+// name exactly, that key wins over keys that match it only once case and
+// underscores are ignored. Objects nested in the conf follow the same rules,
+// including objects in slices and objects that are map values. The keys of a
+// map field stay as written, because they are data rather than field names.
+//
+// Keys that match no field are ignored, because other tasks in the run may
+// read them. A field with no matching key keeps the value target already
+// holds, so set the defaults on target before the call, as Limit does above.
+// The conf carries only the values the run was triggered with: it does not
+// carry the defaults of the Dag's params, so in a scheduled run every field
+// keeps the value it came in with.
+//
+// The `arg:` tag that binds a TaskFlow argument has no effect here. Read a key
+// that no field name can match, such as target-table, from [DagRun.Conf], or
+// name it in a `json:` tag.
+//
+// A number decoded into an any, such as a field of type any or a value of a
+// map[string]any or []any field, arrives as a [json.Number] rather than as the
+// number type [DagRun.Conf] carries, because a float64 cannot hold every
+// integer a conf can. Read it with its Int64 or Float64 method, or give the
+// field a Go integer or float type to have the decoder convert it. Of those,
+// only an integer type keeps every digit of a large integer.
+//
+// It returns an error when a value does not fit the type of its field, when
+// two keys match one field only once case and underscores are ignored, or when
+// two fields of a struct differ only in case or underscores.
+func (d DagRun) UnmarshalConf(target any) error {
+ rv := reflect.ValueOf(target)
+ if rv.Kind() != reflect.Pointer || rv.IsNil() {
+ return fmt.Errorf(
+ "sdk.DagRun.UnmarshalConf: target must be a non-nil
pointer, got %T",
+ target,
+ )
+ }
+ matched, err := matchConfValue(d.Conf, rv.Type().Elem())
+ if err != nil {
+ return err
+ }
+ blob, err := json.Marshal(matched)
+ if err != nil {
+ return fmt.Errorf("sdk.DagRun.UnmarshalConf: cannot encode the
Dag run conf: %w", err)
+ }
+ dec := json.NewDecoder(bytes.NewReader(blob))
+ // A number that reaches an any-typed destination keeps its literal, so
the
+ // task reads an exact integer instead of the float64 encoding/json
would
+ // otherwise decode it into.
+ dec.UseNumber()
+ if err := dec.Decode(target); err != nil {
+ return fmt.Errorf(
+ "sdk.DagRun.UnmarshalConf: cannot read the Dag run conf
into %s: %w",
+ rv.Type().Elem(),
+ err,
+ )
+ }
+ return nil
+}
+
+// matchConfValue renames the keys of every conf object that becomes a struct
+// when the conf is decoded into target. encoding/json matches a key to a field
+// only when the two differ at most in case, so renaming the keys to the exact
+// field names is what lets target_table reach TargetTable.
+//
+// The walk follows target rather than the conf, so it reaches objects nested
+// in slices, arrays and map values, and stops at a type that decodes itself.
+func matchConfValue(value any, target reflect.Type) (any, error) {
+ for target.Kind() == reflect.Pointer {
+ target = target.Elem()
+ }
+ // A type such as time.Time decodes from a value of its own shape, and
its
+ // Go fields are not conf keys.
+ if implementsConfUnmarshaler(target) {
+ return value, nil
+ }
+
+ switch target.Kind() {
+ case reflect.Struct:
+ if obj, ok := value.(map[string]any); ok {
+ return matchConfFields(obj, target)
+ }
+ case reflect.Map:
+ if obj, ok := value.(map[string]any); ok {
+ matched := make(map[string]any, len(obj))
+ for key, elem := range obj {
+ elem, err := matchConfValue(elem, target.Elem())
+ if err != nil {
+ return nil, err
+ }
+ matched[key] = elem
+ }
+ return matched, nil
+ }
+ case reflect.Slice, reflect.Array:
+ if list, ok := value.([]any); ok {
+ matched := make([]any, len(list))
+ for i, elem := range list {
+ elem, err := matchConfValue(elem, target.Elem())
+ if err != nil {
+ return nil, err
+ }
+ matched[i] = elem
+ }
+ return matched, nil
+ }
+ }
+ // A value whose shape does not fit the field, such as a string for a
+ // struct, passes through so that the decoder reports the mismatch.
+ return value, nil
+}
+
+// matchConfFields renames each key of obj to the name of the target field the
+// key matches, and drops a key that matches no field. Keys are visited in
+// sorted order so that an error names the same pair of keys on every run.
+func matchConfFields(obj map[string]any, target reflect.Type) (map[string]any,
error) {
Review Comment:
This copies encoding/json's field rules by hand, so the two can drift apart.
##########
go-sdk/sdk/conf_test.go:
##########
@@ -0,0 +1,301 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package sdk
+
+import (
+ "encoding/json"
+ "testing"
+ "time"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+type etlTable struct {
+ TableName string
+}
+
+type etlSource struct {
+ BucketName string
+}
+
+type etlConf struct {
+ TargetTable string
+ Limit int
+ Since time.Time
+ Source *etlSource
+ Tables []etlTable
+ TableArray [1]etlTable
+ TablesByName map[string]etlTable
+ Options map[string]any
+}
+
+type taggedConf struct {
+ Table string `json:"target_table"`
+ Rows int `json:"row_count,omitempty"`
+ Secret string `json:"-"`
+}
+
+type commonConf struct {
+ Env string
+ Limit int
+}
+
+type jobConf struct {
+ commonConf
+
+ Limit int
+ Name string
+}
+
+type foldClashConf struct {
+ TargetTable string
+ Targettable string
+}
+
+// Tablename folds to the same token as the embedded TableName, which only the
+// `json:"-"` keeps from being a clash.
+type skippedEmbedConf struct {
+ etlTable `json:"-"`
+
+ Tablename string
+}
+
+type untypedConf struct {
+ RowCount int64
+ Options map[string]any
+ Items []any
+ Anything any
+}
+
+func dagRunWithConf(conf map[string]any) DagRun {
+ return DagRun{DagID: "dag1", RunID: "run1", Conf: conf}
+}
+
+func TestUnmarshalConfMatchesKeysIgnoringCaseAndUnderscores(t *testing.T) {
+ var conf etlConf
+
+ require.NoError(t, dagRunWithConf(map[string]any{
+ "target_table": "sales",
+ "LIMIT": int64(5),
+ }).UnmarshalConf(&conf))
+
+ assert.Equal(t, "sales", conf.TargetTable)
+ assert.Equal(t, 5, conf.Limit)
+}
+
+// Another task of the run may read a key this task does not declare, so an
+// unmatched key is not an error, and the field keeps the default the task set.
+func TestUnmarshalConfIgnoresUnmatchedKeysAndKeepsDefaults(t *testing.T) {
+ conf := etlConf{Limit: 100}
+
+ require.NoError(t, dagRunWithConf(map[string]any{
+ "notify_email": "[email protected]",
+ }).UnmarshalConf(&conf))
+
+ assert.Empty(t, conf.TargetTable)
+ assert.Equal(t, 100, conf.Limit)
+}
+
+func TestUnmarshalConfPrefersAnExactKeyOverFoldedOnes(t *testing.T) {
+ var conf etlConf
+
+ require.NoError(t, dagRunWithConf(map[string]any{
+ "target_table": "folded",
+ "TargetTable": "exact",
+ "TARGET_TABLE": "folded",
+ }).UnmarshalConf(&conf))
+
+ assert.Equal(t, "exact", conf.TargetTable)
+}
+
+func TestUnmarshalConfMatchesKeysOfNestedObjects(t *testing.T) {
+ var conf etlConf
+
+ require.NoError(t, dagRunWithConf(map[string]any{
+ "source": map[string]any{"bucket_name": "raw"},
+ "tables": []any{map[string]any{"table_name": "orders"}},
+ "table_array": []any{map[string]any{"table_name": "items"}},
+ "tables_by_name": map[string]any{"Sales_Key":
map[string]any{"table_name": "sales"}},
+ }).UnmarshalConf(&conf))
+
+ require.NotNil(t, conf.Source)
+ assert.Equal(t, "raw", conf.Source.BucketName)
+ assert.Equal(t, []etlTable{{TableName: "orders"}}, conf.Tables)
+ assert.Equal(t, [1]etlTable{{TableName: "items"}}, conf.TableArray)
+ assert.Equal(t, map[string]etlTable{"Sales_Key": {TableName: "sales"}},
conf.TablesByName)
+}
+
+// The keys of a map are data, so they reach the task as the run was triggered
+// with them, at every depth.
+func TestUnmarshalConfKeepsTheKeysOfAMapWithUntypedValues(t *testing.T) {
+ options := map[string]any{
+ "Rows": []any{map[string]any{"Key_A": "a"}},
+ "Inner_Map": map[string]any{"Key_B": "b"},
+ }
+ var conf etlConf
+
+ require.NoError(t, dagRunWithConf(map[string]any{"options":
options}).UnmarshalConf(&conf))
+
+ assert.Equal(t, options, conf.Options)
+}
+
+// The value is the first integer a float64 rounds, so the test also fails if
+// the number passes through a float64 before it becomes a json.Number.
+func TestUnmarshalConfReadsANumberIntoAnUntypedFieldAsAJSONNumber(t
*testing.T) {
+ var conf untypedConf
+
+ require.NoError(t, dagRunWithConf(map[string]any{
+ "row_count": int64(9007199254740993),
+ "options": map[string]any{"rows": int64(9007199254740993)},
+ "items": []any{int64(9007199254740993)},
+ "anything": int64(9007199254740993),
+ }).UnmarshalConf(&conf))
+
+ assert.Equal(t, int64(9007199254740993), conf.RowCount)
+ assert.Equal(t, json.Number("9007199254740993"), conf.Options["rows"])
+ assert.Equal(t, json.Number("9007199254740993"), conf.Items[0])
+ assert.Equal(t, json.Number("9007199254740993"), conf.Anything)
+}
+
+func TestUnmarshalConfReadsATimestampIntoATimeField(t *testing.T) {
+ var conf etlConf
+
+ require.NoError(t, dagRunWithConf(map[string]any{
+ "since": "2024-01-31T02:00:00+08:00",
+ }).UnmarshalConf(&conf))
+
+ assert.True(
+ t,
+ conf.Since.Equal(time.Date(2024, 1, 31, 2, 0, 0, 0,
time.FixedZone("", 8*60*60))),
+ )
+}
+
+func TestUnmarshalConfMatchesAJSONTagAndSkipsASkippedField(t *testing.T) {
+ var conf taggedConf
+
+ require.NoError(t, dagRunWithConf(map[string]any{
+ "TargetTable": "sales",
+ "ROW_COUNT": int64(5),
+ "secret": "token",
+ }).UnmarshalConf(&conf))
+
+ assert.Equal(t, "sales", conf.Table)
+ assert.Equal(t, 5, conf.Rows)
+ assert.Empty(t, conf.Secret, `a json:"-" field takes no conf key`)
+}
+
+func TestUnmarshalConfFillsPromotedFieldsAndHonoursShadowing(t *testing.T) {
+ var conf jobConf
+
+ require.NoError(t, dagRunWithConf(map[string]any{
+ "env": "prod",
+ "limit": int64(5),
+ }).UnmarshalConf(&conf))
+
+ assert.Equal(t, "prod", conf.Env)
+ assert.Equal(t, 5, conf.Limit)
+ assert.Zero(t, conf.commonConf.Limit, "the outer field shadows the one
it promotes")
+}
+
+// encoding/json skips an embedded struct tagged `json:"-"` rather than
+// promoting its fields, so a conf key reaches none of them either.
+func TestUnmarshalConfPromotesNoFieldOfASkippedEmbeddedStruct(t *testing.T) {
+ var conf skippedEmbedConf
+
+ require.NoError(
+ t,
+ dagRunWithConf(map[string]any{"table_name":
"sales"}).UnmarshalConf(&conf),
+ )
+
+ assert.Equal(t, "sales", conf.Tablename)
+ assert.Empty(t, conf.TableName, `a json:"-" embedded struct takes no
conf key`)
+}
+
+func TestUnmarshalConfRejectsTwoKeysThatMatchOneField(t *testing.T) {
+ var conf etlConf
+
+ err := dagRunWithConf(map[string]any{
+ "target_table": "a",
+ "TARGET_TABLE": "b",
+ }).UnmarshalConf(&conf)
+
+ require.EqualError(
+ t,
+ err,
+ `sdk.DagRun.UnmarshalConf: Dag run conf keys "TARGET_TABLE" and
"target_table" both match field `+
+ `etlConf.TargetTable when case and underscores are
ignored, so the SDK cannot tell which one to `+
+ `use; name the field or its json tag exactly as the key
it should take`,
+ )
+}
+
+// The struct, not the conf, is what has to be fixed, so the error does not
+// wait for a run whose conf happens to hold one of the two names.
+func TestUnmarshalConfRejectsFieldsThatDifferOnlyInCaseOrUnderscores(t
*testing.T) {
+ var conf foldClashConf
+
+ err := DagRun{DagID: "dag1", RunID: "run1"}.UnmarshalConf(&conf)
+
+ require.EqualError(
+ t,
+ err,
+ "sdk.DagRun.UnmarshalConf: fields foldClashConf.TargetTable and
foldClashConf.Targettable differ "+
+ "only in case or underscores, so the SDK cannot tell
which one a Dag run conf key should go "+
+ "to; rename one of them",
+ )
+}
+
+func TestUnmarshalConfNamesTheTargetWhenAValueDoesNotFit(t *testing.T) {
+ var conf etlConf
+
+ err := dagRunWithConf(map[string]any{"limit":
"many"}).UnmarshalConf(&conf)
+
+ require.ErrorContains(
+ t,
+ err,
+ "sdk.DagRun.UnmarshalConf: cannot read the Dag run conf into
sdk.etlConf: ",
+ )
+ require.ErrorContains(
+ t,
+ err,
+ "cannot unmarshal string into Go struct field etlConf.Limit of
type int",
+ )
+}
+
+func TestUnmarshalConfRejectsATargetItCannotFill(t *testing.T) {
+ var nilTarget *etlConf
+
+ for _, target := range []any{etlConf{}, nilTarget, nil} {
+ err := dagRunWithConf(map[string]any{"limit":
int64(5)}).UnmarshalConf(target)
+
+ require.ErrorContains(
+ t,
+ err,
+ "sdk.DagRun.UnmarshalConf: target must be a non-nil
pointer, got ",
+ )
+ }
+}
+
+// A run triggered without a conf leaves every field as the task set it.
+func TestUnmarshalConfOfARunWithoutAConf(t *testing.T) {
+ conf := etlConf{Limit: 100}
+
+ require.NoError(t, DagRun{DagID: "dag1", RunID:
"run1"}.UnmarshalConf(&conf))
+
+ assert.Equal(t, etlConf{Limit: 100}, conf)
+}
Review Comment:
Tests for both fixes above. Both fail on the current head.
```suggestion
}
type unexportedEmbedded struct{ Name string }
type withUnexportedPointer struct {
*unexportedEmbedded
Zone string
}
func TestUnmarshalConfSkipsAnUnexportedEmbeddedPointer(t *testing.T) {
var conf withUnexportedPointer
require.NoError(t, dagRunWithConf(map[string]any{
"name": "x",
"zone": "eu",
}).UnmarshalConf(&conf))
assert.Equal(t, "eu", conf.Zone)
assert.Nil(t, conf.unexportedEmbedded)
}
func TestUnmarshalConfOfARunWithoutAConfLeavesANilMapNil(t *testing.T) {
var conf map[string]any
require.NoError(t, DagRun{DagID: "dag1", RunID:
"run1"}.UnmarshalConf(&conf))
assert.Nil(t, conf)
}
```
##########
go-sdk/sdk/conf.go:
##########
@@ -0,0 +1,370 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package sdk
+
+import (
+ "bytes"
+ "encoding"
+ "encoding/json"
+ "fmt"
+ "maps"
+ "reflect"
+ "slices"
+ "strings"
+ "sync"
+)
+
+// UnmarshalConf reads [DagRun.Conf] into target, which must be a non-nil
+// pointer, usually to a struct with one field per conf key:
+//
+// type etlConf struct {
+// TargetTable string
+// Limit int
+// Since time.Time
+// }
+//
+// etl := etlConf{Limit: 100}
+// if err := actx.DagRun().UnmarshalConf(&etl); err != nil {
+// return err
+// }
+//
+// For a run triggered with {"target_table": "sales", "since":
"2024-01-31T00:00:00Z"},
+// etl.TargetTable is "sales", etl.Since is that date, and etl.Limit stays 100.
+//
+// Each key goes to the field whose name matches it when case and underscores
+// are ignored, so target_table reaches TargetTable. A `json:` tag renames the
+// field, and the tag name matches the same way. If one key matches a field
+// name exactly, that key wins over keys that match it only once case and
+// underscores are ignored. Objects nested in the conf follow the same rules,
+// including objects in slices and objects that are map values. The keys of a
+// map field stay as written, because they are data rather than field names.
+//
+// Keys that match no field are ignored, because other tasks in the run may
+// read them. A field with no matching key keeps the value target already
+// holds, so set the defaults on target before the call, as Limit does above.
+// The conf carries only the values the run was triggered with: it does not
+// carry the defaults of the Dag's params, so in a scheduled run every field
+// keeps the value it came in with.
+//
+// The `arg:` tag that binds a TaskFlow argument has no effect here. Read a key
+// that no field name can match, such as target-table, from [DagRun.Conf], or
+// name it in a `json:` tag.
+//
+// A number decoded into an any, such as a field of type any or a value of a
+// map[string]any or []any field, arrives as a [json.Number] rather than as the
+// number type [DagRun.Conf] carries, because a float64 cannot hold every
+// integer a conf can. Read it with its Int64 or Float64 method, or give the
+// field a Go integer or float type to have the decoder convert it. Of those,
+// only an integer type keeps every digit of a large integer.
+//
+// It returns an error when a value does not fit the type of its field, when
+// two keys match one field only once case and underscores are ignored, or when
+// two fields of a struct differ only in case or underscores.
+func (d DagRun) UnmarshalConf(target any) error {
+ rv := reflect.ValueOf(target)
+ if rv.Kind() != reflect.Pointer || rv.IsNil() {
+ return fmt.Errorf(
+ "sdk.DagRun.UnmarshalConf: target must be a non-nil
pointer, got %T",
+ target,
+ )
+ }
+ matched, err := matchConfValue(d.Conf, rv.Type().Elem())
+ if err != nil {
+ return err
+ }
+ blob, err := json.Marshal(matched)
+ if err != nil {
+ return fmt.Errorf("sdk.DagRun.UnmarshalConf: cannot encode the
Dag run conf: %w", err)
+ }
+ dec := json.NewDecoder(bytes.NewReader(blob))
+ // A number that reaches an any-typed destination keeps its literal, so
the
+ // task reads an exact integer instead of the float64 encoding/json
would
+ // otherwise decode it into.
+ dec.UseNumber()
+ if err := dec.Decode(target); err != nil {
+ return fmt.Errorf(
+ "sdk.DagRun.UnmarshalConf: cannot read the Dag run conf
into %s: %w",
+ rv.Type().Elem(),
+ err,
+ )
+ }
+ return nil
+}
+
+// matchConfValue renames the keys of every conf object that becomes a struct
+// when the conf is decoded into target. encoding/json matches a key to a field
+// only when the two differ at most in case, so renaming the keys to the exact
+// field names is what lets target_table reach TargetTable.
+//
+// The walk follows target rather than the conf, so it reaches objects nested
+// in slices, arrays and map values, and stops at a type that decodes itself.
+func matchConfValue(value any, target reflect.Type) (any, error) {
+ for target.Kind() == reflect.Pointer {
+ target = target.Elem()
+ }
+ // A type such as time.Time decodes from a value of its own shape, and
its
+ // Go fields are not conf keys.
+ if implementsConfUnmarshaler(target) {
+ return value, nil
+ }
+
+ switch target.Kind() {
+ case reflect.Struct:
+ if obj, ok := value.(map[string]any); ok {
+ return matchConfFields(obj, target)
+ }
+ case reflect.Map:
+ if obj, ok := value.(map[string]any); ok {
+ matched := make(map[string]any, len(obj))
+ for key, elem := range obj {
+ elem, err := matchConfValue(elem, target.Elem())
+ if err != nil {
+ return nil, err
+ }
+ matched[key] = elem
+ }
+ return matched, nil
+ }
+ case reflect.Slice, reflect.Array:
+ if list, ok := value.([]any); ok {
+ matched := make([]any, len(list))
+ for i, elem := range list {
+ elem, err := matchConfValue(elem, target.Elem())
+ if err != nil {
+ return nil, err
+ }
+ matched[i] = elem
+ }
+ return matched, nil
+ }
+ }
+ // A value whose shape does not fit the field, such as a string for a
+ // struct, passes through so that the decoder reports the mismatch.
+ return value, nil
+}
+
+// matchConfFields renames each key of obj to the name of the target field the
+// key matches, and drops a key that matches no field. Keys are visited in
+// sorted order so that an error names the same pair of keys on every run.
+func matchConfFields(obj map[string]any, target reflect.Type) (map[string]any,
error) {
+ fields, err := confFieldsOf(target)
+ if err != nil {
+ return nil, err
+ }
+
+ matched := make(map[string]any, len(obj))
+ keyOfField := make(map[string]string, len(obj))
+ for _, key := range slices.Sorted(maps.Keys(obj)) {
+ field, ok := fields.byName[key]
+ if !ok {
+ // A key that folds to a field claims that field only
when no other
+ // key names it exactly.
+ folded, found := fields.byFold[foldConfName(key)]
+ if !found {
+ continue
+ }
+ if _, exact := obj[folded.name]; exact {
+ continue
+ }
+ field = folded
+ }
+ if other, dup := keyOfField[field.name]; dup {
+ return nil, fmt.Errorf(
+ "sdk.DagRun.UnmarshalConf: Dag run conf keys %q
and %q both match field %s when case and "+
+ "underscores are ignored, so the SDK
cannot tell which one to use; name the field or "+
+ "its json tag exactly as the key it
should take",
+ other,
+ key,
+ field.goName,
+ )
+ }
+ keyOfField[field.name] = key
+
+ value, err := matchConfValue(obj[key], field.typ)
+ if err != nil {
+ return nil, err
+ }
+ matched[field.name] = value
+ }
+ return matched, nil
+}
+
+// confField is one field a conf key can reach.
+type confField struct {
+ // name is the key the decoder matches the field by: the `json:` tag
name
+ // when the field has one, and the Go field name otherwise.
+ name string
+ // goName is the field as an error names it, qualified with the struct
that
+ // declares it, which is an embedded struct for a promoted field.
+ goName string
+ typ reflect.Type
+}
+
+type confFields struct {
+ byName map[string]confField
+ byFold map[string]confField
+ err error
+}
+
+// Indexing a struct costs far more than renaming the keys of one object of
+// that struct, so a conf holding a list of many objects would otherwise spend
+// most of its time indexing the same struct again.
+var confFieldCache sync.Map // reflect.Type -> *confFields
+
+func confFieldsOf(target reflect.Type) (*confFields, error) {
+ if cached, ok := confFieldCache.Load(target); ok {
+ fields := cached.(*confFields)
+ return fields, fields.err
+ }
+ fields := indexConfFields(target)
+ confFieldCache.Store(target, fields)
+ return fields, fields.err
+}
+
+func indexConfFields(target reflect.Type) *confFields {
+ fields := &confFields{
+ byName: map[string]confField{},
+ byFold: map[string]confField{},
+ }
+ // Breadth-first over embedded structs, because a field of the outer
struct
+ // shadows the field of the same name it promotes, as encoding/json
does.
+ // A struct can embed a pointer to itself, so each one is visited once.
+ visited := map[reflect.Type]bool{target: true}
+ for level := []reflect.Type{target}; len(level) > 0; {
+ var embedded []reflect.Type
+ for _, structType := range level {
+ for _, inner := range collectConfFields(structType,
fields) {
+ if !visited[inner] {
+ visited[inner] = true
+ embedded = append(embedded, inner)
+ }
+ }
+ }
+ if fields.err != nil {
+ return fields
+ }
+ level = embedded
+ }
+ return fields
+}
+
+// collectConfFields adds the fields one struct declares and returns the
+// structs it embeds, whose fields belong to the next level.
+func collectConfFields(structType reflect.Type, fields *confFields)
[]reflect.Type {
+ var embedded []reflect.Type
+ for i := range structType.NumField() {
+ f := structType.Field(i)
+ name, tagged := confFieldName(f)
+ // `json:"-"` on an embedded struct drops the fields it would
promote,
+ // as encoding/json does, so the check comes before the walk
into it.
+ // Promoting them instead can fail a struct the decoder fills,
with an
+ // error naming a field no conf key can reach.
+ if name == "" {
+ continue
+ }
+ if f.Anonymous && !tagged {
+ if inner := confStructType(f.Type); inner != nil {
Review Comment:
An unexported embedded pointer makes every call fail, even for unrelated
fields. Skip it:
```suggestion
if f.Anonymous && !tagged {
// The decoder cannot allocate a pointer to an
unexported struct.
if f.Type.Kind() == reflect.Pointer && !f.IsExported() {
continue
}
if inner := confStructType(f.Type); inner != nil {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
