damccorm commented on a change in pull request #16776:
URL: https://github.com/apache/beam/pull/16776#discussion_r802222444
##########
File path: sdks/go/pkg/beam/core/typex/class.go
##########
@@ -91,16 +92,25 @@ func ClassOf(t reflect.Type) Class {
// data must be fully serializable. Functions and channels are examples of
invalid
// types. Aggregate types with no universals are considered concrete here.
func IsConcrete(t reflect.Type) bool {
+ concrete, _ := isConcrete(t, make(map[uintptr]bool))
+ return concrete
+}
+
+// AssertConcrete returns true iff the given type is a valid "concrete" data
type and if not,
+// returns false along with an error indicating why the type is not concrete.
Concrete
+// data must be fully serializable. Functions and channels are examples of
invalid
+// types. Aggregate types with no universals are considered concrete here.
+func AssertConcrete(t reflect.Type) (bool, error) {
return isConcrete(t, make(map[uintptr]bool))
}
-func isConcrete(t reflect.Type, visited map[uintptr]bool) bool {
+func isConcrete(t reflect.Type, visited map[uintptr]bool) (bool, error) {
Review comment:
That sounds good to me. I considered doing the same in `CheckConcrete`
and just returning an error, but decided to leave it because otherwise we're
technically leaking some (small) responsibility of answering if a type is
concrete to the caller which felt wrong.
##########
File path: sdks/go/pkg/beam/core/typex/class.go
##########
@@ -91,16 +92,25 @@ func ClassOf(t reflect.Type) Class {
// data must be fully serializable. Functions and channels are examples of
invalid
// types. Aggregate types with no universals are considered concrete here.
func IsConcrete(t reflect.Type) bool {
+ concrete, _ := isConcrete(t, make(map[uintptr]bool))
+ return concrete
+}
+
+// AssertConcrete returns true iff the given type is a valid "concrete" data
type and if not,
+// returns false along with an error indicating why the type is not concrete.
Concrete
+// data must be fully serializable. Functions and channels are examples of
invalid
+// types. Aggregate types with no universals are considered concrete here.
+func AssertConcrete(t reflect.Type) (bool, error) {
Review comment:
CheckConcrete works for me!
##########
File path: sdks/go/pkg/beam/core/typex/class.go
##########
@@ -91,16 +92,25 @@ func ClassOf(t reflect.Type) Class {
// data must be fully serializable. Functions and channels are examples of
invalid
// types. Aggregate types with no universals are considered concrete here.
func IsConcrete(t reflect.Type) bool {
+ concrete, _ := isConcrete(t, make(map[uintptr]bool))
+ return concrete
+}
+
+// AssertConcrete returns true iff the given type is a valid "concrete" data
type and if not,
+// returns false along with an error indicating why the type is not concrete.
Concrete
+// data must be fully serializable. Functions and channels are examples of
invalid
+// types. Aggregate types with no universals are considered concrete here.
+func AssertConcrete(t reflect.Type) (bool, error) {
return isConcrete(t, make(map[uintptr]bool))
}
-func isConcrete(t reflect.Type, visited map[uintptr]bool) bool {
+func isConcrete(t reflect.Type, visited map[uintptr]bool) (bool, error) {
Review comment:
That sounds good to me. I considered doing the same in `CheckConcrete`
(previously `AssertConcrete`) and just returning an error, but decided to leave
it because otherwise we're technically leaking some (small) responsibility of
answering if a type is concrete to the caller which felt wrong.
--
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]
