Skip to content

AVRO-2720: Enhance AvroTypeException message to include field name#1156

Closed
subhashb wants to merge 6 commits intoapache:masterfrom
subhashb:2720-display-invalid-field-data
Closed

AVRO-2720: Enhance AvroTypeException message to include field name#1156
subhashb wants to merge 6 commits intoapache:masterfrom
subhashb:2720-display-invalid-field-data

Conversation

@subhashb
Copy link
Copy Markdown
Contributor

The exception message now includes the field name on which the type exception was raised.

Closes: AVRO-2720

Make sure you have checked all steps below.

Jira

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:

Commits

  • My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from the body by a blank line
    2. Subject is limited to 50 characters (not including Jira issue reference)
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Documentation

  • In case of new functionality, my PR adds documentation that describes how to use it.
    NA

The exception message now includes the field name on which the type exception
was raised.

Closes: AVRO-2720
@Fokko Fokko requested a review from kojiromike April 6, 2021 18:19
@subhashb
Copy link
Copy Markdown
Contributor Author

subhashb commented Apr 7, 2021

@kojiromike Addressed review comments.

SchemaResolutionException still accepts named arguments. I thought we can retain it as-is for now. Let me know if you think otherwise.

@subhashb
Copy link
Copy Markdown
Contributor Author

subhashb commented Jul 8, 2021

@kojiromike I have resynced this branch with master and resolved conflicts. There is a flaky test that has failed (test_minimum_speed), otherwise looks good.

@kojiromike
Copy link
Copy Markdown
Contributor

@subhashb I'm still seeing an error in github:

image

@subhashb
Copy link
Copy Markdown
Contributor Author

subhashb commented Jul 8, 2021

Ha! That's strange. I see that this branch has no conflicts.
Is Github showing an older version for you? How many commits do you see? There should be 6 in total.

Avro-1156

@kojiromike
Copy link
Copy Markdown
Contributor

I see six commits, yes. But I still see the error.

image

@subhashb
Copy link
Copy Markdown
Contributor Author

subhashb commented Jul 8, 2021

Hmm, I am not sure what's happening here. I tried merging this branch into master locally, and it worked.
merge-2720-to-master

@kojiromike
Copy link
Copy Markdown
Contributor

@subhashb do the commits listed in github have the same sha hashes as the commits you have locally?

@subhashb
Copy link
Copy Markdown
Contributor Author

subhashb commented Jul 9, 2021

Yep, they seem to. 😕

github-SHA-commits
local-commit-SHAs

@kojiromike
Copy link
Copy Markdown
Contributor

@subhashb if I check out this PR by doing gh pr checkout 1156 (using the github cli) and then rebase master into it, I do see conflicts:

22:38:37 $ gh pr checkout 1156
remote: Enumerating objects: 46, done.
remote: Counting objects: 100% (41/41), done.
remote: Compressing objects: 100% (2/2), done.
remote: Total 46 (delta 39), reused 40 (delta 39), pack-reused 5
Unpacking objects: 100% (46/46), 15.63 KiB | 432.00 KiB/s, done.
From github.com:apache/avro
 * [new ref]             refs/pull/1156/head -> 2720-display-invalid-field-data
Switched to branch '2720-display-invalid-field-data'

22:38:47 $ git status
On branch 2720-display-invalid-field-data
nothing to commit, working tree clean

22:38:49 $ git rebase origin/master
Auto-merging lang/py/avro/test/test_io.py
CONFLICT (content): Merge conflict in lang/py/avro/test/test_io.py
Auto-merging lang/py/avro/io.py
CONFLICT (content): Merge conflict in lang/py/avro/io.py
Auto-merging lang/py/avro/errors.py
CONFLICT (content): Merge conflict in lang/py/avro/errors.py
error: could not apply 6c84ab2af... AVRO-2720: Enhance AvroTypeException message to include field name
Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 6c84ab2af... AVRO-2720: Enhance AvroTypeException message to include field name

@subhashb
Copy link
Copy Markdown
Contributor Author

subhashb commented Jul 9, 2021

Apparently, rebasing the 6 commits into master causes conflicts since they are applied one by one. merge doesn't have this issue because it applies the last state onto master but results in loss of history.

Closing this PR in favor of a clean slate PR #1287.

@subhashb subhashb closed this Jul 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants