Skip to content

Conversation

@hcemk
Copy link
Contributor

@hcemk hcemk commented Mar 9, 2022

Signed-off-by: hcemk [email protected]

Contributes to # (21)

What did you do? upgraded Cloudant library to cloudant-java-sdk

Why did you do it? Because the java-cloudant library is deprecated

How have you tested it? using swagger-ui

Were docs updated if needed?

  • [ x] No
  • Yes
  • N/A

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • [ x] Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new console logs
  • Fixes entire issue
  • Partial fix for issue

@upkarlidder
Copy link
Member

@hcemk good work. I see some conflicts with the main branch. Are you able to resolve the conflicts before we can review and merge? Thank you.

FYI @demilolu

Copy link
Member

@upkarlidder upkarlidder left a comment

Choose a reason for hiding this comment

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

@hcemk good work. I see some conflicts with the main branch. Are you able to resolve the conflicts before we can review and merge? Thank you.

FYI @demilolu

@hcemk
Copy link
Contributor Author

hcemk commented Mar 13, 2022

@hcemk good work. I see some conflicts with the main branch. Are you able to resolve the conflicts before we can review and merge? Thank you.

FYI @demilolu

Hi @upkarlidder I will resolve the conflicts. I just realized the unit tests were build around the old database client, so it might take sometime to resolve and test, but I will try to get that done by mid-week

Signed-off-by: hcemk <[email protected]>
@hcemk
Copy link
Contributor Author

hcemk commented Mar 28, 2022

@upkarlidder I resolved the conflicts but there are issues with the upstream code:

  1. mvn verify is failing. Some test cases expect the server to be running on local host:

Tests run: 5, Failures: 0, Errors: 5, Skipped: 0, Time elapsed: 0.393 sec <<< FAILURE! - in it.CaseIT
getCaseByAttorney_getsCaseByAttorneyAndId(it.CaseIT) Time elapsed: 0.249 sec <<< ERROR!
java.lang.RuntimeException: Server returned HTTP response code: 500 for URL: http://localhost:9080/v1/attorney
Resource class io.swagger.api.AttorneyApi can not be instantiated due to InvocationTargetException
at it.AggregatorConnection.handleException(AggregatorConnection.java:64)
at it.AggregatorConnection.doRequest(AggregatorConnection.java:55)
at it.AggregatorConnection.doPost(AggregatorConnection.java:23)
at it.TestAttorney$Builder.create(TestAttorney.java:112)
at it.CaseIT.addAttorney(CaseIT.java:113)

  1. I used this link for guide into updating the Cloudant library:
    https://github.com/cloudant/java-cloudant/blob/master/MIGRATION.md
    There are a lot of nested calls and each transaction uses a different class to setup parameters.
    To fix the merge conflict from the test class, I tried using RETURNS_DEEP_STUBS to avoid mocking inner classes but that did not work, so I ended up mocking all those inner classes.

  2. The library is still at version 0.0.36. Is this fully production ready ?

@demilolu demilolu requested a review from nitekon1 April 1, 2022 17:30
@demilolu
Copy link
Contributor

demilolu commented Apr 1, 2022

@yochanah can you take a look and provide your insight:

#54 (comment)

@yochanah
Copy link
Collaborator

yochanah commented Apr 5, 2022

ok sure

@upkarlidder upkarlidder requested a review from yochanah April 8, 2022 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

4 participants