avro icon indicating copy to clipboard operation
avro copied to clipboard

AVRO-2075, add flag to SchemaCompatibility to report lossy conversions

Open epkanol opened this issue 6 years ago • 10 comments

epkanol avatar Dec 12 '18 21:12 epkanol

@dkulp, I decided to re-implement the patch, based off of my old branch, as there had been some changes to SchemaCompatibility (plus, my old change had some refactoring changes that made reviews harder than they would have to be). Unit tests pass on my local copy of Avro, but I had some issue with reflect test cases when doing just mvn install (mvn clean install worked) - is this something already known?

epkanol avatar Dec 12 '18 21:12 epkanol

@epkanol did you also run the Python tests:

+ python lang/py/build/src/avro/tool.py rpcsend http://127.0.0.1:39837 share/test/schemas/simple.avpr echo -file share/test/interop/rpc/echo/foo/request.avro
Traceback (most recent call last):
  File "lang/py/build/src/avro/tool.py", line 160, in <module>
    sys.exit(main(sys.argv))
  File "lang/py/build/src/avro/tool.py", line 156, in main
    send_message(uri, proto, msg, datum)
  File "lang/py/build/src/avro/tool.py", line 96, in send_message
    print requestor.request(msg, datum)
  File "/testptch/unknown/lang/py/build/src/avro/ipc.py", line 141, in request
    self.write_call_request(message_name, request_datum, buffer_encoder)
  File "/testptch/unknown/lang/py/build/src/avro/ipc.py", line 180, in write_call_request
    self.write_request(message.request, request_datum, encoder)
  File "/testptch/unknown/lang/py/build/src/avro/ipc.py", line 184, in write_request
    datum_writer.write(request_datum, encoder)
  File "/testptch/unknown/lang/py/build/src/avro/io.py", line 778, in write
    raise AvroTypeException(self.writers_schema, datum)
avro.io.AvroTypeException: The datum {u'record': {u'kind': u'FOO', u'hash': '0123456789012345', u'name': u'Foo'}} is not an example of the schema [
  {
    "type": {
      "javaAnnotation": "org.apache.avro.TestAnnotation", 
      "type": "record", 
      "namespace": "org.apache.avro.test", 
      "name": "TestRecord", 
      "fields": [
        {
          "javaAnnotation": "org.apache.avro.TestAnnotation", 
          "type": "string", 
          "name": "name", 
          "order": "ignore"
        }, 
        {
          "type": {
            "symbols": [
              "FOO", 
              "BAR", 
              "BAZ"
            ], 
            "javaAnnotation": "org.apache.avro.TestAnnotation", 
            "namespace": "org.apache.avro.test", 
            "name": "Kind", 
            "type": "enum"
          }, 
          "name": "kind", 
          "order": "descending"
        }, 
        {
          "type": {
            "javaAnnotation": "org.apache.avro.TestAnnotation", 
            "size": 16, 
            "namespace": "org.apache.avro.test", 
            "name": "MD5", 
            "type": "fixed"
          }, 
          "name": "hash"
        }
      ]
    }, 
    "name": "record"
  }
]

Fokko avatar Dec 13 '18 08:12 Fokko

? I did not - but my patch didn't change anything in the python code? Does the python code use the Java classes?

epkanol avatar Dec 13 '18 08:12 epkanol

There are RPC tests which do calls from different languages, among others from Python/Java/Ruby, to make sure that the different implementations are still compatible: https://api.travis-ci.org/v3/job/467201360/log.txt

Fokko avatar Dec 13 '18 08:12 Fokko

I'm running the tests locally as we speak, but the change I made in SchemaCompatibility should be backwards compatible (essentially I just added a boolean, defaulting to the old behaviour), so if my patch fails also locally, I will re-run the tests off regular master as well. Will post the results here.

epkanol avatar Dec 13 '18 08:12 epkanol

@epkanol My bad, master is failing.

Fokko avatar Dec 13 '18 09:12 Fokko

Please rebase onto latest master to fix the CI.

Fokko avatar Dec 26 '18 20:12 Fokko

Done, now awaiting Travis verdict (no conflicts in rebase)

epkanol avatar Dec 26 '18 21:12 epkanol

@Fokko - seems like the csharp tests are failing in my PR - but I have not changed anything in the cs code base, and only made a small change in the SchemaCompatibility class (not in any real marshalling code). Workflow: fetch and merge "epkanol:master" onto avro/master rebase my original patch onto epkanol:master (creating a new commit) - no conflicts detected by git force-push the new commit onto my git repo (which has the PR towards avro/master) - essentially losing the older commit, but the newer commit is the one we want to merge anyway, and it contains all relevant changes...

The force-pushing does trigger Travis, so I would expect this work flow to be acceptable to github Still, the csharp build seems to fail Where did I go wrong? I never did any csharp changes? Building java locally (mvn clean install) works successfully

epkanol avatar Dec 27 '18 07:12 epkanol

We introduced automatic code formatting. For more info see the "How to contribute" page. This probably affected this PR can you please rebase it.

iemejia avatar Mar 26 '19 15:03 iemejia