Added deocde flag to get_channel_config_with_orderer #105
Conversation
nobody4t
left a comment
nobody4t
left a comment
There was a problem hiding this comment.
- why we need this encoded config? Is there any scenario for this? Perhaps I missed some.
- If this function return encoded config, then we do not need to decode it at all. So logic here should be modified. If decoding is unnecceary, do not decode it.
Please verify the first question before moving on.
|
@dongwangdw, I've created this PR because of #103, there is an issue with decoding of the config. So at least having an option fo fetch encoded config and decode it later with some other tool is helpful. Decoding was left there because someone might already use it, in addition function |
| break | ||
|
|
||
| block = BlockDecoder().decode(block.SerializeToString()) | ||
| decoded_block = BlockDecoder().decode(block.SerializeToString()) |
There was a problem hiding this comment.
When decode==False, do we still needs to run the decode(...)?
There was a problem hiding this comment.
in this case, it seems like we do, since after that it checks whether the correct block was fetched and fetches another one if needed.
OK. There is a data missing that you mentioned in #103. I would like to advise to investigate it deep for the root cause. And maybe a fix for it. It is very hard to fix it then let's have an easy workaround here. And your proposal would make more sense. Again I so much appreciated your PR. |
|
@dongwangdw,I've already tried looking into the cause of this issue, but with no luck. This PR was already the last resort. Proto files look almost identical for this python-sdk and https://github.com/hyperledger/fabric-protos, especially for the Block message. Although I'm neither really familiar with the internals of this SDK nor have any experience with protobufs and might be missing something so if someone who is more familiar with SDK could take a look would be great. |
…n ability to fetch config without decoding Signed-off-by: nkalichynskyi <ext-nazarii.kalichynskyi@here.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
CI is not happy, please check the result and fix: https://github.com/hyperledger/fabric-sdk-py/pull/105/checks?check_run_id=1012418829. |
Signed-off-by: nkalichynskyi <ext-nazarii.kalichynskyi@here.com>
@yeasy, Found what's wrong with tests. The latest tag is missing now for fabric-ccenv image, that what caused tests to fail. I've fixed this by setting in the docker compose files same version as in check-env.sh. |
* Added deocde flag to get_channel_config_with_orderer method to have an ability to fetch config without decoding Signed-off-by: nkalichynskyi <ext-nazarii.kalichynskyi@here.com> * fixed issue with missing latest tag for fabric-ccenv image Signed-off-by: nkalichynskyi <ext-nazarii.kalichynskyi@here.com>
4 participants
Added deocde flag to
get_channel_config_with_orderermethod to have an ability to fetch config without decoding