Skip to content

Issue 2311: Adds support to decode metadata in the API that list ledgers#2312

Merged
eolivelli merged 7 commits intoapache:masterfrom
fantapsody:decode_metadata
May 27, 2020
Merged

Issue 2311: Adds support to decode metadata in the API that list ledgers#2312
eolivelli merged 7 commits intoapache:masterfrom
fantapsody:decode_metadata

Conversation

@fantapsody
Copy link
Contributor

@fantapsody fantapsody commented Apr 18, 2020

Descriptions of the changes in this PR:

Motivation

The current list ledgers API output the metadata in a serialized binary format, which is not friendly to human operators and external tools, and is not consistent with the output of the API that gets the metadata.

Changes

The PR adds a parameter called decode_meta, and output the ledger metadata in decoded format when the parameter presents and the value of it is 'true'.

Master Issue: #2311


In order to uphold a high standard for quality for code contributions, Apache BookKeeper runs various precommit
checks for pull requests. A pull request can only be merged when it passes precommit checks.


Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

If this PR is a BookKeeper Proposal (BP):

  • Make sure the PR title is formatted like:
    <BP-#>: Description of bookkeeper proposal
    e.g. BP-1: 64 bits ledger is support
  • Attach the master issue link in the description of this PR.
  • Attach the google doc link if the BP is written in Google Doc.

Otherwise:

  • Make sure the PR title is formatted like:
    <Issue #>: Description of pull request
    e.g. Issue 123: Description ...
  • Make sure tests pass via mvn clean apache-rat:check install spotbugs:check.
  • Replace <Issue #> in the title with the actual Issue number.

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM

But please add a test

@fantapsody fantapsody requested a review from eolivelli April 20, 2020 23:08
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM
we have fixed the problem on Integration tests.
This patch is now good to go.

Let's wait for another binding +1 before merging the patch to master

@sijie sijie added this to the 4.11.0 milestone Apr 21, 2020
@eolivelli
Copy link
Contributor

can you please merge with current master?

@fantapsody
Copy link
Contributor Author

can you please merge with current master?

The page shows that it has to be someone with write access can merge the PR...

@eolivelli
Copy link
Contributor

Sorry
I meant to rebase this patch onto current master.
Two ways:
git checkout xxxxx
git fetch apache master

git merge apache/master
Or
git rebase apache/master

Assuming that xxxx is your branch and 'apache' is your git remote repository for main apache Bookkeeper repository

@fantapsody
Copy link
Contributor Author

Sorry
I meant to rebase this patch onto current master.
Two ways:
git checkout xxxxx
git fetch apache master

git merge apache/master
Or
git rebase apache/master

Assuming that xxxx is your branch and 'apache' is your git remote repository for main apache Bookkeeper repository

Sure, I have rebased the PR, please take a look.

@eolivelli
Copy link
Contributor

I have restarted integration tests

@eolivelli eolivelli merged commit 5233027 into apache:master May 27, 2020
Ghatage pushed a commit to Ghatage/bookkeeper that referenced this pull request Jun 19, 2020
…st ledgers

Descriptions of the changes in this PR:

### Motivation

The current list ledgers API output the metadata in a serialized binary format, which is not friendly to human operators and external tools, and is not consistent with the output of the [API that gets the metadata](https://bookkeeper.apache.org/docs/4.9.2/admin/http/#endpoint-apiv1ledgermetadataledger_idledger_id).

### Changes

The PR adds a parameter called `decode_meta`, and output the ledger metadata in decoded format when the parameter presents and the value of it is 'true'.

Master Issue: apache#2311



Reviewers: Sijie Guo <None>, Enrico Olivelli <eolivelli@gmail.com>

This closes apache#2312 from fantapsody/decode_metadata, closes apache#2311
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants