[jira] [Comment Edited] (AVRO-1329) Get per-symbol doc for enums
[ https://issues.apache.org/jira/browse/AVRO-1329?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17103989#comment-17103989 ] Felix GV edited comment on AVRO-1329 at 5/11/20, 1:06 AM: -- Order is definitely important in the already-existing symbols array, but that does not necessarily mean that the metadata collection need be ordered in the same way (i.e. as a parallel array) nor that it meeds to be dense (i.e. a map structure keyed by symbol name would allow documenting only some of the symbols, just like record field documentation today). It is valid to design the metadata as a parallel array, of course, but regardless of the metadata shape, I assume the spec would instruct implementations to use the ordinal corresponding to the symbols array index. was (Author: felixgv): Order is definitely important in the already-existing symbols array, but that does not necessarily mean that the metadata collection need be ordered in the same way (i.e. as a parallel array) nor that it meeds to be dense (i.e. a map structure keyed by symbol name would allow documenting only some of the fields, just like record field documentation today). It is valid to design the metadata as a parallel array, of course, but regardless of the metadata shape, I assume the spec would instruct implementations to use the ordinal corresponding to the symbols array index. > Get per-symbol doc for enums > > > Key: AVRO-1329 > URL: https://issues.apache.org/jira/browse/AVRO-1329 > Project: Apache Avro > Issue Type: Improvement > Components: doc >Affects Versions: 1.7.4 >Reporter: Felix GV >Priority: Minor > > It would be nice to have the ability to add documentation for each symbol of > an enum. > Doug Cutting, quoted from the mailing list: > Documentation per enum symbol is not currently supported, but would not be > difficult to add. Please file an issue in Jira if you'd like to see this. For > compatibility, in Json, this would probably appear as a parallel array of > documentation strings, e.g., something like: > ("name": "Foo", "type":"enum", "doc":"an enum", "symbols":["X","Y"], > "symbols-doc":["X is X", "Y is Y"]} -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (AVRO-1329) Get per-symbol doc for enums
[ https://issues.apache.org/jira/browse/AVRO-1329?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17103989#comment-17103989 ] Felix GV commented on AVRO-1329: Order is definitely important in the already-existing symbols array, but that does not necessarily mean that the metadata collection need be ordered in the same way (i.e. as a parallel array) nor that it meeds to be dense (i.e. a map structure keyed by symbol name would allow documenting only some of the fields, just like record field documentation today). It is valid to design the metadata as a parallel array, of course, but regardless of the metadata shape, I assume the spec would instruct implementations to use the ordinal corresponding to the symbols array index. > Get per-symbol doc for enums > > > Key: AVRO-1329 > URL: https://issues.apache.org/jira/browse/AVRO-1329 > Project: Apache Avro > Issue Type: Improvement > Components: doc >Affects Versions: 1.7.4 >Reporter: Felix GV >Priority: Minor > > It would be nice to have the ability to add documentation for each symbol of > an enum. > Doug Cutting, quoted from the mailing list: > Documentation per enum symbol is not currently supported, but would not be > difficult to add. Please file an issue in Jira if you'd like to see this. For > compatibility, in Json, this would probably appear as a parallel array of > documentation strings, e.g., something like: > ("name": "Foo", "type":"enum", "doc":"an enum", "symbols":["X","Y"], > "symbols-doc":["X is X", "Y is Y"]} -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (AVRO-1329) Get per-symbol doc for enums
[ https://issues.apache.org/jira/browse/AVRO-1329?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17103987#comment-17103987 ] radai rosenblatt commented on AVRO-1329: enums are still encoded as their ordinals in binary format (which is the only format avro is truly compatible in). this means symbol order is important - so i'd stick with an array over a map. > Get per-symbol doc for enums > > > Key: AVRO-1329 > URL: https://issues.apache.org/jira/browse/AVRO-1329 > Project: Apache Avro > Issue Type: Improvement > Components: doc >Affects Versions: 1.7.4 >Reporter: Felix GV >Priority: Minor > > It would be nice to have the ability to add documentation for each symbol of > an enum. > Doug Cutting, quoted from the mailing list: > Documentation per enum symbol is not currently supported, but would not be > difficult to add. Please file an issue in Jira if you'd like to see this. For > compatibility, in Json, this would probably appear as a parallel array of > documentation strings, e.g., something like: > ("name": "Foo", "type":"enum", "doc":"an enum", "symbols":["X","Y"], > "symbols-doc":["X is X", "Y is Y"]} -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [avro] anhldbk commented on a change in pull request #874: AVRO-2808: provide details while encountering non-static inner classes
anhldbk commented on a change in pull request #874: URL: https://github.com/apache/avro/pull/874#discussion_r422725600 ## File path: lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java ## @@ -737,6 +737,9 @@ protected Schema createSchema(Type type, Map names) { AvroName annotatedName = field.getAnnotation(AvroName.class); // Rename fields String fieldName = (annotatedName != null) ? annotatedName.value() : field.getName(); + if ("this$0".equals(fieldName)) { Review comment: @Fokko agree. Do you have any suggestion for the constant name? Is `STRING_OUTER_PARENT_REFERENCE` ok? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [avro] siad007 opened a new pull request #878: AVRO-2752: PHP: Setup as Composer package on Packagist.org
siad007 opened a new pull request #878: URL: https://github.com/apache/avro/pull/878 Added composer setup. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Comment Edited] (AVRO-1329) Get per-symbol doc for enums
[ https://issues.apache.org/jira/browse/AVRO-1329?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17103927#comment-17103927 ] Felix GV edited comment on AVRO-1329 at 5/10/20, 7:32 PM: -- Hindsight being 20/20, having syntactic coherence between record fields and enum metadata does sound appealing. However, if an incompatible schema spec change is under consideration, then making field definition in a map-style rather than an array-style fashion is theoretically also on the table... Personally, I think (or rather, hope) that this ship has sailed. In my opinion, having a 2.0 version at all would be a monumental failure. The one thing that a serialization spec ought to do is maintain compatibility as religiously as possible. I would argue that any fully-incompatible changes (i.e. not even backwards compatible) should be a whole new project/fork, rather than parading as a 2.0 version of a project that intrinsically should never have a major version bump. We can call it Captain Avro or whatever. So essentially, my point is that there are 2 paths: 1) We consider backwards incompatible changes unacceptable, which means the symbols array will remain forever no matter what, and we need to make the best of the hand we’ve been dealt in terms of defining symbol metadata. 2) We consider incompatible changes acceptable, in which case, changes can be defined in any part of the spec (i.e. record field definition is also eligible for a facelift, not just enum definition). Personally, I prefer working under model (1), but I would be curious to hear the rest of the community’s opinion. Also, any plan we cook up should take into account not just the AVSC spec but also the IDL one. was (Author: felixgv): Hindsight being 20/20, having syntactic coherence between record fields and enum metadata does sound appealing. However, if an incompatible schema spec change is under consideration, then making field definition in a map-style rather than an array-style fashion is theoretically also on the table... Personally, I think (or rather, hope) that this ship has sailed. In my opinion, having a 2.0 version at all would be a monumental failure. The one thing that a serialization spec ought to do is maintain compatibility as religiously as possible. I would argue that any fully-incompatible changes (i.e. not even backwards compatible) should be a whole new project/fork, rather than parading as a 2.0 version of a project that intrinsically should never have a major version bump. We can call it Captain Avro or whatever. So essentially, my point is that there are 2 paths: 1) We consider backwards incompatible changes unacceptable, which means the symbols array will remain forever no matter what, and we need to make the best of the hand we’ve been dealt in terms of defining symbol metadata. 2) We consider incompatible changes acceptable, in which case, changes can be defined in any part of the spec (i.e. record field definition is also eligible for a facelift, not just enum definition). Personally, I prefer working under model (1), but I would be curious to hear the rest of the community’s opinion. Also, any plan we cook up should take into account not just the ACSC spec but also the IDL one. > Get per-symbol doc for enums > > > Key: AVRO-1329 > URL: https://issues.apache.org/jira/browse/AVRO-1329 > Project: Apache Avro > Issue Type: Improvement > Components: doc >Affects Versions: 1.7.4 >Reporter: Felix GV >Priority: Minor > > It would be nice to have the ability to add documentation for each symbol of > an enum. > Doug Cutting, quoted from the mailing list: > Documentation per enum symbol is not currently supported, but would not be > difficult to add. Please file an issue in Jira if you'd like to see this. For > compatibility, in Json, this would probably appear as a parallel array of > documentation strings, e.g., something like: > ("name": "Foo", "type":"enum", "doc":"an enum", "symbols":["X","Y"], > "symbols-doc":["X is X", "Y is Y"]} -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Comment Edited] (AVRO-1329) Get per-symbol doc for enums
[ https://issues.apache.org/jira/browse/AVRO-1329?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17103927#comment-17103927 ] Felix GV edited comment on AVRO-1329 at 5/10/20, 7:28 PM: -- Hindsight being 20/20, having syntactic coherence between record fields and enum metadata does sound appealing. However, if an incompatible schema spec change is under consideration, then making field definition in a map-style rather than an array-style fashion is theoretically also on the table... Personally, I think (or rather, hope) that this ship has sailed. In my opinion, having a 2.0 version at all would be a monumental failure. The one thing that a serialization spec ought to do is maintain compatibility as religiously as possible. I would argue that any fully-incompatible changes (i.e. not even backwards compatible) should be a whole new project/fork, rather than parading as a 2.0 version of a project that intrinsically should never have a major version bump. We can call it Captain Avro or whatever. So essentially, my point is that there are 2 paths: 1) We consider backwards incompatible changes unacceptable, which means the symbols array will remain forever no matter what, and we need to make the best of the hand we’ve been dealt in terms of defining symbol metadata. 2) We consider incompatible changes acceptable, in which case, changes can be defined in any part of the spec (i.e. record field definition is also eligible for a facelift, not just enum definition). Personally, I prefer working under model (1), but I would be curious to hear the rest of the community’s opinion. Also, any plan we cook up should take into account not just the ACSC spec but also the IDL one. was (Author: felixgv): Hindsight being 20/20, having syntactic coherence between record fields and enum metadata does sound appealing. However, if an incompatible schema spec change is under consideration, then making field definition in a map-style rather than an array-style is theoretically also on the table... Personally, I think (or rather, hope) that this ship has sailed. In my opinion, having a 2.0 version at all would be a monumental failure. The one thing that a serialization spec ought to do is maintain compatibility as religiously as possible. I would argue that any fully-incompatible changes (i.e. not even backwards compatible) should be a whole new project/fork, rather than parading as a 2.0 version of a project that intrinsically should never have a major version bump. We can call it Captain Avro or whatever. So essentially, my point is that there are 2 paths: 1) We consider backwards incompatible changes unacceptable, which means the symbols array will remain forever no matter what, and we need to make the best of the hand we’ve been dealt in terms of defining symbol metadata. 2) We consider incompatible changes acceptable, in which case, changes can be defined in any part of the spec (i.e. record field definition is also eligible for a facelift, not just enum definition). Personally, I prefer working under model (1), but I would be curious to hear the rest of the community’s opinion. Also, any plan we cook up should take into account not just the ACSC spec but also the IDL one. > Get per-symbol doc for enums > > > Key: AVRO-1329 > URL: https://issues.apache.org/jira/browse/AVRO-1329 > Project: Apache Avro > Issue Type: Improvement > Components: doc >Affects Versions: 1.7.4 >Reporter: Felix GV >Priority: Minor > > It would be nice to have the ability to add documentation for each symbol of > an enum. > Doug Cutting, quoted from the mailing list: > Documentation per enum symbol is not currently supported, but would not be > difficult to add. Please file an issue in Jira if you'd like to see this. For > compatibility, in Json, this would probably appear as a parallel array of > documentation strings, e.g., something like: > ("name": "Foo", "type":"enum", "doc":"an enum", "symbols":["X","Y"], > "symbols-doc":["X is X", "Y is Y"]} -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (AVRO-1329) Get per-symbol doc for enums
[ https://issues.apache.org/jira/browse/AVRO-1329?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17103927#comment-17103927 ] Felix GV commented on AVRO-1329: Hindsight being 20/20, having syntactic coherence between record fields and enum metadata does sound appealing. However, if an incompatible schema spec change is under consideration, then making field definition in a map-style rather than an array-style is theoretically also on the table... Personally, I think (or rather, hope) that this ship has sailed. In my opinion, having a 2.0 version at all would be a monumental failure. The one thing that a serialization spec ought to do is maintain compatibility as religiously as possible. I would argue that any fully-incompatible changes (i.e. not even backwards compatible) should be a whole new project/fork, rather than parading as a 2.0 version of a project that intrinsically should never have a major version bump. We can call it Captain Avro or whatever. So essentially, my point is that there are 2 paths: 1) We consider backwards incompatible changes unacceptable, which means the symbols array will remain forever no matter what, and we need to make the best of the hand we’ve been dealt in terms of defining symbol metadata. 2) We consider incompatible changes acceptable, in which case, changes can be defined in any part of the spec (i.e. record field definition is also eligible for a facelift, not just enum definition). Personally, I prefer working under model (1), but I would be curious to hear the rest of the community’s opinion. Also, any plan we cook up should take into account not just the ACSC spec but also the IDL one. > Get per-symbol doc for enums > > > Key: AVRO-1329 > URL: https://issues.apache.org/jira/browse/AVRO-1329 > Project: Apache Avro > Issue Type: Improvement > Components: doc >Affects Versions: 1.7.4 >Reporter: Felix GV >Priority: Minor > > It would be nice to have the ability to add documentation for each symbol of > an enum. > Doug Cutting, quoted from the mailing list: > Documentation per enum symbol is not currently supported, but would not be > difficult to add. Please file an issue in Jira if you'd like to see this. For > compatibility, in Json, this would probably appear as a parallel array of > documentation strings, e.g., something like: > ("name": "Foo", "type":"enum", "doc":"an enum", "symbols":["X","Y"], > "symbols-doc":["X is X", "Y is Y"]} -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (AVRO-1329) Get per-symbol doc for enums
[ https://issues.apache.org/jira/browse/AVRO-1329?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17103911#comment-17103911 ] Martin Jubelgas commented on AVRO-1329: --- I had considered a map, too, but having declarations for enum values similar to declarations to fields of an object felt better to me, but I'm open to suggestions, that's why I am tossing ideas here before implementing anything. To me, it felt like, ideally, one should replace the "symbols" field with a more capable, extendable structure, but since just replacing the string with a JSON object will most likely be unacceptable for reasons of backwards compatibility, I'd basically leave in the "symbols" only to retain the backwards compatibility, to maybe be removed in something like AVRO 2.0 and move the imperative information to another field. (schema validation would have to make sure that both sources of information are conclusive with each other) So basically, there's a couple of options: * for each extra information for the enums, add another parallel array (such as suggested by Doug in the original post), which I personally find the least charming way of doing it * add the extended information in one parallel array of json objects/maps (maybe the best compromise) * add the extended information in a map that optionally assigns extra information to respective symbols * define the symbols and their extensions in an array (which I consider would be the best solution if starting from a and clean slate) and keep the old array for backward compatibility I haven't quite decided on what path I find preferrable, all things considered. Seems like each comes with its quirks. > Get per-symbol doc for enums > > > Key: AVRO-1329 > URL: https://issues.apache.org/jira/browse/AVRO-1329 > Project: Apache Avro > Issue Type: Improvement > Components: doc >Affects Versions: 1.7.4 >Reporter: Felix GV >Priority: Minor > > It would be nice to have the ability to add documentation for each symbol of > an enum. > Doug Cutting, quoted from the mailing list: > Documentation per enum symbol is not currently supported, but would not be > difficult to add. Please file an issue in Jira if you'd like to see this. For > compatibility, in Json, this would probably appear as a parallel array of > documentation strings, e.g., something like: > ("name": "Foo", "type":"enum", "doc":"an enum", "symbols":["X","Y"], > "symbols-doc":["X is X", "Y is Y"]} -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [avro] Fokko commented on a change in pull request #874: AVRO-2808: provide details while encountering non-static inner classes
Fokko commented on a change in pull request #874: URL: https://github.com/apache/avro/pull/874#discussion_r422680754 ## File path: lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java ## @@ -737,6 +737,9 @@ protected Schema createSchema(Type type, Map names) { AvroName annotatedName = field.getAnnotation(AvroName.class); // Rename fields String fieldName = (annotatedName != null) ? annotatedName.value() : field.getName(); + if ("this$0".equals(fieldName)) { Review comment: For readability. By splitting out this constant we can name the variable: https://en.wikipedia.org/wiki/Magic_number_(programming) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [avro] siad007 commented on a change in pull request #876: AVRO-2751: Upgrade Compatibility to PHP 7.4
siad007 commented on a change in pull request #876: URL: https://github.com/apache/avro/pull/876#discussion_r422677904 ## File path: lang/php/lib/avro/data_file.php ## @@ -209,12 +209,12 @@ class AvroDataIOReader /** * @var string */ - private $sync_marker; + public $sync_marker; Review comment: Without this change the code would fail on line 460 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [avro] siad007 commented on pull request #875: AVRO-2828: Add AvroNotImplementedException
siad007 commented on pull request #875: URL: https://github.com/apache/avro/pull/875#issuecomment-626364475 I created a ticket [here](https://issues.apache.org/jira/browse/AVRO-2828?jql=project%20%3D%20AVRO%20AND%20component%20%3D%20php). 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [avro] Fokko commented on a change in pull request #876: AVRO-2751: Upgrade Compatibility to PHP 7.4
Fokko commented on a change in pull request #876: URL: https://github.com/apache/avro/pull/876#discussion_r422677098 ## File path: lang/php/lib/avro/data_file.php ## @@ -209,12 +209,12 @@ class AvroDataIOReader /** * @var string */ - private $sync_marker; + public $sync_marker; Review comment: I think we still want to keep this private, as we don't want to expose this outside the object. WDYT? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Commented] (AVRO-2527) Upgrade PHP version to 7.x
[ https://issues.apache.org/jira/browse/AVRO-2527?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17103891#comment-17103891 ] Fokko Driesprong commented on AVRO-2527: [~siad007] feel free to open up a PR to bump the version in the CI > Upgrade PHP version to 7.x > -- > > Key: AVRO-2527 > URL: https://issues.apache.org/jira/browse/AVRO-2527 > Project: Apache Avro > Issue Type: Improvement > Components: php >Reporter: Kengo Seki >Assignee: Kengo Seki >Priority: Major > Fix For: 1.10.0 > > > Avro currently supports PHP 5.x, but [its support period has expired on Jan > 2019|https://www.php.net/supported-versions.php]. > We should support PHP 7.1+, on which the community support is continuing at > this time. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (AVRO-1329) Get per-symbol doc for enums
[ https://issues.apache.org/jira/browse/AVRO-1329?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17103842#comment-17103842 ] Felix GV commented on AVRO-1329: Wow, what a throwback. I didn’t remember submitting this nearly 7 years ago Regarding the latest spec proposal, it seems mildly awkward to me to have the spec be a parallel array of objects that contain the symbol name. A parallel array shouldn’t need to contain the symbol since its index is itself the symbol reference. Alternatively, if having the symbol next to the doc is considered better for readability, then why not use a map (i.e. JSON object) instead, where the key is the symbol name and the value is a doc string, or another object containing a doc string field? This would mean the schema parser can be tolerant of out of order doc or sparse doc, since the map provides a strong guarantee that each symbol will be present only once. -F > Get per-symbol doc for enums > > > Key: AVRO-1329 > URL: https://issues.apache.org/jira/browse/AVRO-1329 > Project: Apache Avro > Issue Type: Improvement > Components: doc >Affects Versions: 1.7.4 >Reporter: Felix GV >Priority: Minor > > It would be nice to have the ability to add documentation for each symbol of > an enum. > Doug Cutting, quoted from the mailing list: > Documentation per enum symbol is not currently supported, but would not be > difficult to add. Please file an issue in Jira if you'd like to see this. For > compatibility, in Json, this would probably appear as a parallel array of > documentation strings, e.g., something like: > ("name": "Foo", "type":"enum", "doc":"an enum", "symbols":["X","Y"], > "symbols-doc":["X is X", "Y is Y"]} -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (AVRO-1329) Get per-symbol doc for enums
[ https://issues.apache.org/jira/browse/AVRO-1329?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17103741#comment-17103741 ] Martin Jubelgas commented on AVRO-1329: --- [~anhldbk] - good point. think the spec change is within the scope of this issue? as in implementation, I am volunteering for the java part, but for other languages, other people would have to contribute. (but, since the change is kind of an "optional extension" for now and leaves everything else working, it shouldn't be a biggie if language support isn't at 100% initally) > Get per-symbol doc for enums > > > Key: AVRO-1329 > URL: https://issues.apache.org/jira/browse/AVRO-1329 > Project: Apache Avro > Issue Type: Improvement > Components: doc >Affects Versions: 1.7.4 >Reporter: Felix GV >Priority: Minor > > It would be nice to have the ability to add documentation for each symbol of > an enum. > Doug Cutting, quoted from the mailing list: > Documentation per enum symbol is not currently supported, but would not be > difficult to add. Please file an issue in Jira if you'd like to see this. For > compatibility, in Json, this would probably appear as a parallel array of > documentation strings, e.g., something like: > ("name": "Foo", "type":"enum", "doc":"an enum", "symbols":["X","Y"], > "symbols-doc":["X is X", "Y is Y"]} -- This message was sent by Atlassian Jira (v8.3.4#803005)