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.

Sign up for GitHub

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

Jump to bottom

Bug in the generated client code regarding TimeConstraints #84

Closed
andredasilvapinto opened this issue Oct 25, 2014 · 2 comments
Closed

Bug in the generated client code regarding TimeConstraints #84

andredasilvapinto opened this issue Oct 25, 2014 · 2 comments
Labels
accepted bug
Milestone
Release 3.1.1

Comments

@andredasilvapinto
Copy link
Contributor

The timeout to TimeConstraint conversion is not considering the case of no timeout (timeout == 0) for the AsyncClient.

cougar-codegen-plugin/templates/client-impl.ftl
DefaultTimeConstraints.fromTimeout(timeoutMillis)

This effectively transforms a "no timeout" to a "timeout now" operation.

My suggestion (contained on the referenced pull request) would be not to add the same logic as we have on /templates/clientSyncServiceImpl.ftl, but instead extract it to to a centralized point inside the fromTimeout static method.
@andredasilvapinto andredasilvapinto mentioned this issue Oct 25, 2014
Merged
eswdd added a commit that referenced this issue Oct 28, 2014
@eswdd
Merge pull request #85 from andredasilvapinto/fix_time_constraints
14b7297 
#84 Fixing calculation of TimeConstraints based on timeouts for Async clients
@eswdd eswdd added this to the Release 3.2.0 milestone Oct 28, 2014
@eswdd eswdd closed this as completed Oct 28, 2014
@andredasilvapinto andredasilvapinto mentioned this issue Nov 17, 2014
Closed
@eswdd eswdd reopened this Nov 17, 2014
@eswdd
Copy link
Contributor

eswdd commented Nov 17, 2014

We need to expand integration tests so that this would have been caught

@eswdd eswdd modified the milestones: Release 3.2.0, Release 3.1.1 Nov 18, 2014
@eswdd eswdd mentioned this issue Dec 2, 2014
Closed
@eswdd
Copy link
Contributor

eswdd commented Jan 25, 2015

Will expand tests in trunk under #91

@eswdd eswdd closed this as completed Jan 25, 2015
@eswdd eswdd mentioned this issue Jan 25, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
accepted bug
Projects
None yet
Milestone
Release 3.1.1
Development

No branches or pull requests
2 participants
@eswdd @andredasilvapinto