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

feat(debugapi, postage): topup batch handling #2401

Merged
merged 13 commits into from
Aug 23, 2021
Merged

feat(debugapi, postage): topup batch handling #2401

merged 13 commits into from
Aug 23, 2021

Conversation

aloknerurkar
Copy link
Contributor

@aloknerurkar aloknerurkar commented Aug 9, 2021 •
edited by acud
Loading

  • Add new API endpoint for topup of batches
  • Add handling in postagecontract.Interface for TopUp
  • Update stampissuer details on topup

This change is Reviewable

@aloknerurkar aloknerurkar marked this pull request as draft August 9, 2021 21:41
@aloknerurkar aloknerurkar marked this pull request as ready for review August 10, 2021 09:38
@aloknerurkar aloknerurkar requested review from acud and zelig August 10, 2021 09:39
@aloknerurkar aloknerurkar self-assigned this Aug 11, 2021
@acud
Copy link
Member

acud commented Aug 11, 2021

@aloknerurkar could you please also update the openapi spec to reflect these changes?

@aloknerurkar
Copy link
Contributor Author

@aloknerurkar could you please also update the openapi spec to reflect these changes?

Yes. I was hoping @zelig to review once and approve the endpoint path and format.

aloknerurkar
aloknerurkar commented Aug 12, 2021
View reviewed changes
pkg/postage/postagecontract/contract_test.go Show resolved Hide resolved
pkg/postage/service.go Outdated Show resolved Hide resolved
pkg/postage/postagecontract/contract.go Show resolved Hide resolved
acud
acud previously requested changes Aug 12, 2021
View reviewed changes
Copy link
Member

@acud acud left a comment

Choose a reason for hiding this comment

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

the endpoint is on the wrong api :)

pkg/api/postage.go Outdated Show resolved Hide resolved
pkg/postage/batchservice/batchservice_test.go Outdated Show resolved Hide resolved
pkg/postage/postagecontract/contract.go Show resolved Hide resolved
pkg/api/router.go Outdated Show resolved Hide resolved
pkg/api/postage.go Outdated Show resolved Hide resolved
pkg/postage/service.go Outdated Show resolved Hide resolved
@aloknerurkar aloknerurkar changed the title feat(api, postage): topup batch handling feat(debugapi, postage): topup batch handling Aug 13, 2021
@aloknerurkar aloknerurkar requested a review from acud August 13, 2021 07:35
@aloknerurkar aloknerurkar added the ready for review The PR is ready to be reviewed label Aug 16, 2021
acud
acud reviewed Aug 16, 2021
View reviewed changes
Copy link
Member

@acud acud left a comment

Choose a reason for hiding this comment

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

Very nice, I have a few minor nitpicks.
Also, a beekeeper integration test that checks that the batch value increases by the amount that was requested would be needed too

openapi/SwarmDebug.yaml Outdated Show resolved Hide resolved
openapi/SwarmDebug.yaml Outdated Show resolved Hide resolved
pkg/debugapi/postage.go Show resolved Hide resolved
@aloknerurkar aloknerurkar dismissed acud’s stale review August 16, 2021 12:34

Changes are done

@aloknerurkar aloknerurkar requested a review from acud August 16, 2021 13:01
@aloknerurkar
Copy link
Contributor Author

@acud Will add beekeeper tests for both topup and dilute in separate beekeeper PR.

ralph-pichler
ralph-pichler suggested changes Aug 17, 2021
View reviewed changes
pkg/postage/postagecontract/contract.go Outdated Show resolved Hide resolved
pkg/postage/postagecontract/contract.go Outdated Show resolved Hide resolved
@aloknerurkar aloknerurkar merged commit 81988b1 into master Aug 23, 2021
@aloknerurkar aloknerurkar deleted the topup.0 branch August 23, 2021 09:33
@acud acud added this to the v1.2.0 milestone Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acud acud acud approved these changes

@ralph-pichler ralph-pichler ralph-pichler approved these changes

@zelig zelig Awaiting requested review from zelig

Assignees

@aloknerurkar aloknerurkar

Labels
pull-request ready for review The PR is ready to be reviewed
Projects
None yet
Milestone
v1.2.0
Development

Successfully merging this pull request may close these issues.
3 participants
@aloknerurkar @acud @ralph-pichler