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

Include error message for JSON processing exception #2271

Merged
merged 2 commits into from
Dec 1, 2022

Conversation

pawel-big-lebowski
Copy link
Collaborator

@pawel-big-lebowski pawel-big-lebowski commented Nov 29, 2022

Signed-off-by: Pawel Leszczynski leszczynski.pawel@gmail.com

Problem

When using a python client, a RunEvent does not validate a date time provided as a string. It's pretty easy to make an error like:

event = RunEvent(
        RunState.START,
        "2020-01-01", # this should be a date time, preferably with a timezone
        Run("123e4567-e89b-12d3-a456-426614174000"),
        Job("openlineage", "job"),
        "http://producer"
    )

c = OpenLineageClient(url=<<some-url>>)
c.emit(event=event)

which returns:

'{"code":400,"message":"Unable to process JSON"}'

which could be more meaningful to find a cause of an issue.

Following improvements should be done:

Closes: #2270

Solution

  • in case of JsonProcessingException, Marquez should include exception in returned error message.

Note: All database schema changes require discussion. Please link the issue for context.

Checklist

  • You've signed-off your work
  • Your changes are accompanied by tests (if relevant)
  • Your change contains a small diff and is self-contained
  • [] You've updated any relevant documentation (if relevant)
  • You've updated the CHANGELOG.md with details about your change under the "Unreleased" section (if relevant, depending on the change, this may not be necessary)
  • You've versioned your .sql database schema migration according to Flyway's naming convention (if relevant)
  • You've included a header in any source code files (if relevant)

@boring-cyborg boring-cyborg bot added the api API layer changes label Nov 29, 2022
@codecov
Copy link

codecov bot commented Nov 29, 2022

Codecov Report

Merging #2271 (457cd9a) into main (dedfe48) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #2271      +/-   ##
============================================
+ Coverage     76.74%   76.82%   +0.07%     
- Complexity     1150     1154       +4     
============================================
  Files           219      220       +1     
  Lines          5260     5268       +8     
  Branches        423      423              
============================================
+ Hits           4037     4047      +10     
+ Misses          748      747       -1     
+ Partials        475      474       -1     
Impacted Files Coverage Δ
api/src/main/java/marquez/MarquezContext.java 85.89% <100.00%> (+0.18%) ⬆️
.../api/exceptions/JsonProcessingExceptionMapper.java 100.00% <100.00%> (ø)
...rc/main/java/marquez/logging/LoggingMdcFilter.java 75.00% <0.00%> (+12.50%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pawel-big-lebowski pawel-big-lebowski changed the title provide reason for json processing exception Include error message JSON processing exception Nov 30, 2022
@pawel-big-lebowski pawel-big-lebowski changed the title Include error message JSON processing exception Include error message for JSON processing exception Nov 30, 2022
Signed-off-by: Pawel Leszczynski <leszczynski.pawel@gmail.com>
@boring-cyborg boring-cyborg bot added the docs label Nov 30, 2022
@pawel-big-lebowski pawel-big-lebowski marked this pull request as ready for review November 30, 2022 07:18
@pawel-big-lebowski pawel-big-lebowski self-assigned this Nov 30, 2022
@pawel-big-lebowski pawel-big-lebowski merged commit 36e94ce into main Dec 1, 2022
@pawel-big-lebowski pawel-big-lebowski deleted the json-processing-exception branch December 1, 2022 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api API layer changes docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Json parsing exception with no explanation provided
2 participants