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

"pin verify". #3843

Merged
merged 4 commits into from May 25, 2017

Conversation

Projects
None yet
4 participants
@kevina
Contributor

kevina commented Mar 29, 2017

No description provided.

@kevina kevina added the in progress label Mar 29, 2017

@kevina

This comment has been minimized.

Contributor

kevina commented Mar 29, 2017

This needs work, but want to ge tthe U.I down first. Here is the current usage:

$ ipfs pin verify
QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn ok
QmVLDAhCY3X9P2uRudKAryuQFPM5zqA3Yij1dY8FpGbL7T ok
QmQZ1WVkddYPpRPm6vPpemhNacjwhupzSMzFEcYMisVpuY broken
  QmSijovevteoY63Uj1uC5b8pkpDU5Jgyk2dYBqz3sMJUPc: merkledag: not found
  QmTbPEyrA1JyGUHFvmtx1FNZVzdBreMv8Hc8jV9sBRWhNA: The block referred to by 'QmTbPEyrA1JyGUHFvmtx1FNZVzdBreMv8Hc8jV9sBRWhNA' was not a vald merkledag node
@@ -414,6 +436,90 @@ func pinLsAll(typeStr string, ctx context.Context, n *core.IpfsNode) (map[string
return keys, nil
}
type pinStatus struct {
// use pointer to array slice to reduce memory usage

This comment has been minimized.

@whyrusleeping

whyrusleeping Mar 29, 2017

Member

How does this reduce memory usage?

This comment has been minimized.

@whyrusleeping

whyrusleeping Mar 29, 2017

Member

Just make it a normal slice. Slices are already pointers

This comment has been minimized.

@kevina

kevina Mar 29, 2017

Contributor

A slice is the width of three pointers. There is a node for every indirect pin. By using an extra lawyer of indirection I reduce the size by two pointers for each node. The actual array slice will be shared many times in the common case.

return out
}
func (r pinVerifyRes) Format(out io.Writer) {

This comment has been minimized.

@whyrusleeping

whyrusleeping Mar 29, 2017

Member

Use a marshaler for this.

}
}()
res.SetOutput(rdr)

This comment has been minimized.

@whyrusleeping

whyrusleeping Mar 29, 2017

Member

try not to ever set the output to a reader, that makes the http api realllllly hard to deal with

This comment has been minimized.

@kevina

kevina Mar 29, 2017

Contributor

@whyrusleeping this is a WIP, more concerned with the U.I. will fix up this later.

@kevina

This comment has been minimized.

Contributor

kevina commented Mar 30, 2017

Here is the current usage:

USAGE
  ipfs pin verify - Verify that recursive pins are complete.

SYNOPSIS
  ipfs pin verify [--verbose] [--quiet | -q]

OPTIONS

  --verbose        bool - Also write the hashes of non-broken pins.
  -q,      --quiet bool - Write just hashes of broken pins.

Here as some sample outputs:

$ ipfs pin verify
QmQZ1WVkddYPpRPm6vPpemhNacjwhupzSMzFEcYMisVpuY broken
  QmSijovevteoY63Uj1uC5b8pkpDU5Jgyk2dYBqz3sMJUPc: merkledag: not found
  QmTbPEyrA1JyGUHFvmtx1FNZVzdBreMv8Hc8jV9sBRWhNA: The block referred to by 'QmTbPEyrA1JyGUHFvmtx1FNZVzdBreMv8Hc8jV9sBRWhNA' was not a vald merkledag node
$ ipfs pin verify -q
QmQZ1WVkddYPpRPm6vPpemhNacjwhupzSMzFEcYMisVpuY
$ ipfs pin verify --verbose
QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn ok
QmVLDAhCY3X9P2uRudKAryuQFPM5zqA3Yij1dY8FpGbL7T ok
QmQZ1WVkddYPpRPm6vPpemhNacjwhupzSMzFEcYMisVpuY broken
  QmSijovevteoY63Uj1uC5b8pkpDU5Jgyk2dYBqz3sMJUPc: merkledag: not found
  QmTbPEyrA1JyGUHFvmtx1FNZVzdBreMv8Hc8jV9sBRWhNA: The block referred to by 'QmTbPEyrA1JyGUHFvmtx1FNZVzdBreMv8Hc8jV9sBRWhNA' was not a vald merkledag node

@whyrusleeping @Kubuxu (and others) let me know if you think the usage and output is reasonable and I will work on adding test cases and fixing the API.

@kevina kevina requested review from whyrusleeping and Kubuxu Apr 26, 2017

@kevina

This comment has been minimized.

Contributor

kevina commented Apr 26, 2017

@whyrusleeping @Kubuxu (and others) let me know if you think the usage and output is reasonable and I will work on adding test cases and fixing the API.

@whyrusleeping whyrusleeping requested review from lgierth and hsanjuan Apr 28, 2017

@hsanjuan

This comment has been minimized.

Contributor

hsanjuan commented Apr 28, 2017

Is this a client side operation only, not exposed through http api?

@kevina

This comment has been minimized.

Contributor

kevina commented Apr 28, 2017

@hsanjuan what makes you say that?

@hsanjuan

This comment has been minimized.

Contributor

hsanjuan commented Apr 28, 2017

maybe just ignorance @kevina. Just not clear to me what the output would be in JSON since structs have private fields.

@kevina

This comment has been minimized.

Contributor

kevina commented Apr 28, 2017

Just not clear to me what the output would be in JSON since structs have private fields.
@hsanjuan that part is not designed yet

At this point I just want to know "if you think the usage and output is reasonable".

This is not ready for a full review yet.

@hsanjuan

This comment has been minimized.

Contributor

hsanjuan commented Apr 28, 2017

@kevina yeah, so this would involve knowing if the JSON output is reasonable, and how it matches the CLI-formatted output etc..

@kevina

This comment has been minimized.

Contributor

kevina commented Apr 28, 2017

I was hoping for some feedback on the U.I. before I concern myself with the API and the JSON output.

@whyrusleeping

This comment has been minimized.

Member

whyrusleeping commented Apr 29, 2017

Yeah, given that the output looks nontrivial (nested things and different sorts of behaviour) it would probably be good to spec out what the api looks like here as that may be just as important as the output.

That said, I think the given output looks good to me. It provides decent enough information to the end user

@kevina

This comment has been minimized.

Contributor

kevina commented May 2, 2017

@hsanjuan @whyrusleeping I fixed the API.

@hsanjuan

This comment has been minimized.

Contributor

hsanjuan commented May 4, 2017

@kevina thanks, can you post the json output for the examples above?

@kevina

This comment has been minimized.

Contributor

kevina commented May 4, 2017

@hsanjuan I would if I knew an easy easy way to get at it. I will hand construct it later today if necessary.

@Kubuxu

This comment has been minimized.

Member

Kubuxu commented May 8, 2017

@kevina ipfs --enc=json pin verify.

@kevina

This comment has been minimized.

Contributor

kevina commented May 8, 2017

Standard output:

> ipfs --enc=json pin verify
{"Cid":"zdvgq3v8bUfT3EskMzK7FqzpMiymfmTLfZA8JZh43wr9RMk64","Ok":false,"BadNodes":[{"Cid":"zdvgq3v8bUfT3EskMzK7FqzpMiymfmTLfZA8JZh43wr9RMk64","Err":"unrecognized object type: %!s(uint64=114)"}]}
...
{"Cid":"QmRKmNZqwSxcPs6HjEN3JiAKxjEJMByYpgr5ykwKuqNahP","Ok":false,"BadNodes":[{"Cid":"zdvgqAiZHaP4GiRxrhecMdw6rb2wBMrP7r2CGbicCznebfLTH","Err":"unrecognized object type: %!s(uint64=114)"},{"Cid":"zdvgqAiZHaP4GiRxrhecMdw6rb2wBMrP7r2CGbicCznebfLTH","Err":"unrecognized object type: %!s(uint64=114)"},{"Cid":"zdvgqAiZHaP4GiRxrhecMdw6rb2wBMrP7r2CGbicCznebfLTH","Err":"unrecognized object type: %!s(uint64=114)"},{"Cid":"zdvgqAiZHaP4GiRxrhecMdw6rb2wBMrP7r2CGbicCznebfLTH","Err":"unrecognized object type: %!s(uint64=114)"}]}

With --quiet option.

> ipfs --enc=json pin verify -q
{"Cid":"zdvgqDYeRDUp9EFkTwt4MDH4GFsZKu5wAA2x5zxcPYxGNZks6","Ok":false}
{"Cid":"QmRKmNZqwSxcPs6HjEN3JiAKxjEJMByYpgr5ykwKuqNahP","Ok":false}

With `--verbose option.

{"Cid":"QmYAUTWVubcjipibdrwkJPbH61XNDrERHyAABRqwWU9K46","Ok":true}
...
{"Cid":"zdvgq3v8bUfT3EskMzK7FqzpMiymfmTLfZA8JZh43wr9RMk64","Ok":false,"BadNodes":[{"Cid":"zdvgq3v8bUfT3EskMzK7FqzpMiymfmTLfZA8JZh43wr9RMk64","Err":"unrecognized object type: %!s(uint64=114)"}]}
...
{"Cid":"QmRKmNZqwSxcPs6HjEN3JiAKxjEJMByYpgr5ykwKuqNahP","Ok":false,"BadNodes":[{"Cid":"zdvgqAiZHaP4GiRxrhecMdw6rb2wBMrP7r2CGbicCznebfLTH","Err":"unrecognized object type: %!s(uint64=114)"},{"Cid":"zdvgqAiZHaP4GiRxrhecMdw6rb2wBMrP7r2CGbicCznebfLTH","Err":"unrecognized object type: %!s(uint64=114)"},{"Cid":"zdvgqAiZHaP4GiRxrhecMdw6rb2wBMrP7r2CGbicCznebfLTH","Err":"unrecognized object type: %!s(uint64=114)"},{"Cid":"zdvgqAiZHaP4GiRxrhecMdw6rb2wBMrP7r2CGbicCznebfLTH","Err":"unrecognized object type: %!s(uint64=114)"}]}
@whyrusleeping

This comment has been minimized.

Member

whyrusleeping commented May 25, 2017

@kevina that api output looks good to me

kevina added some commits Mar 29, 2017

"pin verify": basic implementation
License: MIT
Signed-off-by: Kevin Atkinson <k@kevina.org>
"pin verify": don't use a pointer to a slice.
License: MIT
Signed-off-by: Kevin Atkinson <k@kevina.org>
"pin verify": add --verbose and --quiet options
License: MIT
Signed-off-by: Kevin Atkinson <k@kevina.org>

@kevina kevina changed the title from WIP "pin verify". to "pin verify". May 25, 2017

"pin verify": fix API
License: MIT
Signed-off-by: Kevin Atkinson <k@kevina.org>
@kevina

This comment has been minimized.

Contributor

kevina commented May 25, 2017

@whyrusleeping rebased

@kevina kevina referenced this pull request May 25, 2017

Closed

"pin verify" #3

@whyrusleeping

This LGTM, thanks @kevina !

@whyrusleeping whyrusleeping merged commit c7462ac into master May 25, 2017

6 of 8 checks passed

codecov/patch 0% of diff hit (target 63.93%)
Details
codecov/project 63.7% (-0.24%) compared to 4cf046c
Details
ci/circleci Your tests passed on CircleCI!
Details
codeclimate no new or fixed issues
Details
commit-message-check/gitcop All commit messages are valid
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@whyrusleeping whyrusleeping added this to the Ipfs 0.4.10 milestone May 25, 2017

@whyrusleeping whyrusleeping deleted the kevina/pin-verify branch May 25, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment