Skip to content

Conversation

@honnix
Copy link
Member

@honnix honnix commented Oct 27, 2023

TL;DR

Minor clean up of struct and blob

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

Mostly getting unit tests in place. I didn't do this in previous PR because we did not realize the test cases are in a dedicated maven module.

Tracking Issue

NA

Follow-up issue

NA

durations().asInstanceOf[SdkLiteralType[T]]
case t if t =:= typeOf[Blob] =>
blobs(BlobType.DEFAULT).asInstanceOf[SdkLiteralType[T]]
case t if t <:< typeOf[Product] && !(t =:= typeOf[Option[_]]) =>
Copy link
Member Author

Choose a reason for hiding this comment

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

Kinda best effort matching, not at all precise. I'm wondering whether this method has to be public as it is not used by anything but test case.

jf.Function.identity()
)
case SimpleType.STRUCT => ??? // TODO not yet supported
case SimpleType.STRUCT =>
Copy link
Member Author

@honnix honnix Oct 27, 2023

Choose a reason for hiding this comment

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

Be explicit that we do not support this intentionally.

Support Scala case class instance to auto-value object (vice versa) is not trivial and it would introduce dependency on flytekit-jackson in one way or another. Hopefully in practice we don't really have need of this.

Signed-off-by: Hongxin Liang <honnix@users.noreply.github.com>
@honnix honnix merged commit 8ec7a78 into master Nov 17, 2023
@honnix honnix deleted the more-on-struct branch November 17, 2023 14:34
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.

3 participants