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

resolve #1907 | handle record references when presenting avro schema #1911

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MiloszKorman
Copy link

No description provided.

@@ -190,7 +190,7 @@
"topics": [
{
"id": "pl.allegro.public.group.DummyEvent",
"schema": "{\"type\":\"record\",\"name\":\"DummyEvent\",\"namespace\":\"pl.allegro.public.group.DummyEvent\",\"doc\":\"Event emitted when notification is sent to a user\",\"fields\":[{\"name\":\"__metadata\",\"type\":[\"null\",{\"type\":\"map\",\"values\":\"string\"}],\"doc\":\"Field internally used by Hermes to propagate metadata.\",\"default\":null},{\"name\":\"waybillId\",\"type\":[\"null\",{\"type\":\"record\",\"name\":\"WaybillId\",\"fields\":[{\"name\":\"waybill\",\"type\":\"string\",\"doc\":\"Waybill\"},{\"name\":\"carrierId\",\"type\":\"string\",\"doc\":\"CarrierId\"}]}],\"doc\":\"WaybillId\",\"default\":null},{\"name\":\"notificationId\",\"type\":\"string\",\"doc\":\"Notification Id\"},{\"name\":\"userId\",\"type\":\"string\",\"doc\":\"User Id\"}]}",
"schema": "{\"type\":\"record\",\"name\":\"DummyEvent\",\"namespace\":\"pl.allegro.public.group.DummyEvent\",\"doc\":\"Event emitted when notification is sent to a user\",\"fields\":[{\"name\":\"__metadata\",\"type\":[\"null\",{\"type\":\"map\",\"values\":\"string\"}],\"doc\":\"Field internally used by Hermes to propagate metadata.\",\"default\":null},{\"name\":\"waybillId\",\"type\":[\"null\",{\"type\":\"record\",\"name\":\"WaybillId\",\"fields\":[{\"name\":\"waybill\",\"type\":\"string\",\"doc\":\"Waybill\"},{\"name\":\"carrierId\",\"type\":\"string\",\"doc\":\"CarrierId\"}]}],\"doc\":\"WaybillId\",\"default\":null},{\"name\":\"otherWaybillId\",\"type\":[\"null\",\"WaybillId\"],\"doc\":\"other WaybillId\",\"default\":null},{\"name\":\"notificationId\",\"type\":\"string\",\"doc\":\"Notification Id\"},{\"name\":\"userId\",\"type\":\"string\",\"doc\":\"User Id\"}]}",
Copy link
Author

@MiloszKorman MiloszKorman Oct 16, 2024

Choose a reason for hiding this comment

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

Unsure if this change and feature are significant enough to warrant adding new field with such record type reference to the DummyEvent, or should I just work with this more complex type locally, but merge to master with DummyEvent schema unchanged

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is, it may be helpful in case of any debug related to this feature

@MiloszKorman MiloszKorman force-pushed the issue-1907-show-referenced-record-type branch 9 times, most recently from 01013df to a4c3f37 Compare October 16, 2024 14:46
const isEnum = types.some((type: Type) => type.includes('enum'));
const expandable = isRecord || isEnum;
const nestedType = isRecord && findNestedType(props.field.type);
const nestedType =
isRecord && (recordReference || findNestedType(props.field.type));
Copy link
Author

Choose a reason for hiding this comment

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

Example schemas rendered

Copy link
Author

Choose a reason for hiding this comment

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

{
 "type": "record",
 "name": "Example1",
 "doc": "root",
 "fields": [
   {
     "name": "a",
     "type": ["null", {
       "type": "record",
       "name": "A1",
       "fields": [
         {
           "name": "b",
           "type": ["null", "string"],
           "doc": "b"
         }
       ],
       "doc": "a"
     }]
   },
   {
     "name": "c",
     "type": ["null", "A1"],
     "doc": "c"
   }
 ]
}
image

Copy link
Author

Choose a reason for hiding this comment

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

{
 "type": "record",
 "name": "Example2",
 "namespace": "a.b.c",
 "doc": "root",
 "fields": [
   {
     "name": "a",
     "type": ["null", {
       "type": "record",
       "name": "A2",
       "fields": [
         {
           "name": "b",
           "type": ["null", "string"],
           "doc": "b"
         }
       ],
       "doc": "a"
     }]
   },
   {
     "name": "c",
     "type": ["null", "A2"],
     "doc": "c"
   }
 ]
}
image

Copy link
Author

Choose a reason for hiding this comment

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

{
 "type": "record",
 "name": "Example3",
 "namespace": "a.b.c",
 "doc": "root",
 "fields": [
   {
     "name": "a",
     "type": ["null", {
       "type": "record",
       "name": "A3",
       "fields": [
         {
           "name": "b",
           "type": ["null", "string"],
           "doc": "b"
         }
       ],
       "doc": "a"
     }]
   },
   {
     "name": "c",
     "type": ["null", "a.b.c.A3"],
     "doc": "c"
   }
 ]
}
image

Copy link
Author

Choose a reason for hiding this comment

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

{
 "type": "record",
 "name": "Example4",
 "namespace": "a.b.c",
 "doc": "root",
 "fields": [
   {
     "name": "a",
     "type": ["null", {
       "type": "record",
       "name": "A4",
       "namespace": "d.e.f",
       "fields": [
         {
           "name": "b",
           "type": ["null", "string"],
           "doc": "b"
         }
       ],
       "doc": "a"
     }]
   },
   {
     "name": "c",
     "type": ["null", "d.e.f.A4"],
     "doc": "c"
   }
 ]
}
image

Copy link
Author

Choose a reason for hiding this comment

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

{
 "type": "record",
 "name": "Example5",
 "namespace": "a.b.c",
 "doc": "root",
 "fields": [
   {
     "name": "a",
     "type": ["null", {
       "type": "record",
       "name": "d.e.f.A5",
       "fields": [
         {
           "name": "b",
           "type": ["null", "string"],
           "doc": "b"
         }
       ],
       "doc": "a"
     }]
   },
   {
     "name": "c",
     "type": ["null", "d.e.f.A5"],
     "doc": "c"
   }
 ]
}
image

Copy link
Author

Choose a reason for hiding this comment

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

{
 "type": "record",
 "name": "Example6",
 "namespace": "a.b.c",
 "doc": "root",
 "fields": [
   {
     "name": "a",
     "type": ["null", {
       "type": "record",
       "name": "a.b.c.A6",
       "fields": [
         {
           "name": "b",
           "type": ["null", "string"],
           "doc": "b"
         }
       ],
       "doc": "a"
     }]
   },
   {
     "name": "c",
     "type": ["null", "A6"],
     "doc": "c"
   }
 ]
}
image

Copy link
Author

Choose a reason for hiding this comment

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

{
 "type": "record",
 "name": "Example7",
 "namespace": "a.b.c",
 "doc": "root",
 "fields": [
   {
     "name": "a",
     "type": ["null", {
       "type": "record",
       "name": "a.b.c.A7",
       "fields": [
         {
           "name": "b",
           "type": ["null", "string"],
           "doc": "b"
         }
       ],
       "doc": "a"
     }]
   },
   {
     "name": "c",
     "type": ["null", "a.b.c.A7"],
     "doc": "c"
   }
 ]
}
image

Copy link
Author

Choose a reason for hiding this comment

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

{
 "type": "record",
 "name": "Example8",
 "namespace": "a.b.c",
 "doc": "root",
 "fields": [
   {
     "name": "a",
     "type": ["null", {
       "type": "record",
       "name": "A8",
       "namespace": "d.e.f",
       "fields": [
         {
           "name": "b",
           "type": ["null",
             {
               "type": "record",
               "name": "B8",
               "fields": [
                 {
                   "name": "c",
                   "type": ["null", "string"],
                   "doc": "c"
                 }
               ],
               "doc": "b"
             }],
           "doc": "b"
         }
       ],
       "doc": "a"
     }]
   },
   {
     "name": "d",
     "type": ["null", "d.e.f.B8"],
     "doc": "d"
   }
 ]
}
image

Copy link
Author

Choose a reason for hiding this comment

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

{
 "type": "record",
 "name": "Example9",
 "namespace": "a.b.c",
 "doc": "root",
 "fields": [
   {
     "name": "a",
     "type": ["null", {
       "type": "record",
       "name": "A9",
       "namespace": "d.e.f",
       "fields": [
         {
           "name": "b",
           "type": ["null",
             {
               "type": "record",
               "name": "B9",
               "namespace": "a.b.c",
               "fields": [
                 {
                   "name": "c",
                   "type": ["null", "string"],
                   "doc": "c"
                 }
               ],
               "doc": "b"
             }],
           "doc": "b"
         }
       ],
       "doc": "a"
     }]
   },
   {
     "name": "d",
     "type": ["null", "B9"],
     "doc": "d"
   }
 ]
}
image

@MiloszKorman MiloszKorman force-pushed the issue-1907-show-referenced-record-type branch from a4c3f37 to 8e81aa0 Compare October 16, 2024 21:36
@MiloszKorman MiloszKorman marked this pull request as ready for review October 16, 2024 21:38
@faderskd faderskd force-pushed the issue-1907-show-referenced-record-type branch from 8e81aa0 to c3df9f3 Compare November 14, 2024 16:52
@faderskd
Copy link
Contributor

faderskd commented Nov 14, 2024

Thanks for broad set of screen examples, it helped a lot ;) I think that future code readers will appreciate the link to the rules for resolving references. Pls add link to the avro specification. I guess you've used this one? :D https://avro.apache.org/docs/1.11.1/specification/#names

faderskd
faderskd previously approved these changes Nov 14, 2024
@faderskd faderskd force-pushed the issue-1907-show-referenced-record-type branch 2 times, most recently from 2a33634 to 7b97173 Compare November 25, 2024 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants