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

upgrade cloudant library to cloudant-java-sdk #21 #54

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

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

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