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.

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

Prefixed model names in generated swagger spec #525

Closed
lukasmalkmus opened this issue Jan 16, 2018 · 2 comments
Closed

Prefixed model names in generated swagger spec #525

lukasmalkmus opened this issue Jan 16, 2018 · 2 comments

Comments

@lukasmalkmus
Copy link
Contributor

Why are the models in the swagger spec prefixed with the last bit of the proto package name? Is there a reason for this? I find this pretty ugly tbh and don’t see the advantage.

@lukasmalkmus lukasmalkmus changed the title Prefixed model name sin generated swagger spec Prefixed model names in generated swagger spec Jan 16, 2018
@achew22
Copy link
Collaborator

achew22 commented Feb 16, 2018

The reason to generate the prefixes is that we don't have a guaranteed unique namespace. If two packages produce different Foo messages then we will have trouble.

Why not strip the prefix if it isn't necessary? (the next obvious question)

I tried that, but the problem is that if you do that then when you add a message in that happens to conflict it will break code that is very far away from the code that changed. This is in an effort to adhere to the principle of least astonishment.

If you disagree with me, I would love to hear your reasoning why. Maybe we will even change it to work the other way! I'm happy to answer any more questions you might have about the project. Since I've gotten this question a couple of times in various ways, I'm also going to add a FAQ entry list to the README.md in the root. Feel free to send PRs to enhance that (even if it is only a question, I can propose a change on top of your PR that has the answer and we can merge it!)

achew22 added a commit that referenced this issue Feb 16, 2018
@achew22 achew22 mentioned this issue Feb 16, 2018
@lukasmalkmus
Copy link
Contributor Author

lukasmalkmus commented Feb 16, 2018

Thanks for answering. The answer was so obvious and I still missed it 🤣 Probably because I never encountered the problems you mentioned. But I totally understand you and the solution makes sense to me. However, I think one has design flaws in his protobuf definitions if he/she has multiple messages with the same base message name (even through imports). But I could be wrong, just because it hasn't happened to me, doesn't mean such cases are not present.

We could possibly integrate the ability to handle both cases (with or without prefix) but one the other hand (and since no one besides me has complained yet) it might be the simplest solution to just patch the files by hand with a short script (sed, etc.). But I'm also open for suggestions.

ithinker1991 pushed a commit to tronprotocol/grpc-gateway that referenced this issue Apr 26, 2018
…saction

refactor the transaction structure
ithinker1991 added a commit to tronprotocol/grpc-gateway that referenced this issue May 11, 2018
e92f847 Merge pull request grpc-ecosystem#46 from tronprotocol/rate_limit
e191aed add grpc return type:server busy
c98bb68 Merge pull request grpc-ecosystem#45 from tronprotocol/add_expiration
5abd2c0 fix spell error
76d2ac1 Merge pull request grpc-ecosystem#44 from tronprotocol/add-reason-enum
cf23e93 Merge pull request grpc-ecosystem#42 from tronprotocol/add_witness_committee
e9b0876 add the reason code
cc24179 Merge pull request grpc-ecosystem#43 from tronprotocol/add_expiration
5f9e263 add transaction expiration erro
3913331 fix merge conflict
bb1f7e0 add message Vote
4e0f34e Merge pull request grpc-ecosystem#40 from tronprotocol/proxy
7134384 minor change
ab77170 add witness and committee in account
8f7655a add error code for broadcast.
4bc508c fix build error.
a568eb8 add response code for return of broad_cast && fix error.
4dc6857 add response code for return of broad_cast.
df2ea4b Merge pull request grpc-ecosystem#41 from tronprotocol/add-reason-enum
54dac0c fix-reason
200ba66 proxy support GET method
0e65a14 Merge pull request grpc-ecosystem#39 from tronprotocol/evan-rm-createaccount
d57ad8f remove createaccount api
e13211c Merge pull request grpc-ecosystem#38 from tronprotocol/evan-rm-transaction-type
8aeba00 rm transaction type
c7495f8 Merge pull request grpc-ecosystem#36 from tronprotocol/add-reason-enum
59333a3 add reason
9aa4212 Merge pull request grpc-ecosystem#35 from tronprotocol/add_fullNode_API
e4025ff feat: delete GetTransactionByLimitPrev.
07d94e4 Merge pull request grpc-ecosystem#34 from tronprotocol/add_fullNode_API
884b666 fix: add http option.
5d93c0c feat: add fullnode GRPC.
08b0663 Merge commit 'c3b07944ee8eb47d7461fd304520364db41671ad' into develop
c3b0794 Merge pull request grpc-ecosystem#33 from tronprotocol/add_frozen_allowance
b7224ed add bandwidth in account
3464107 Merge branch 'develop' of https://github.com/tronprotocol/java-tron into develop
9313380 Merge pull request grpc-ecosystem#32 from tronprotocol/add_frozen_allowance
88da776 minor change
f12e2cc minor change
d18ae9c minor change
e18bdf2 add frozen and allowance balance
479564d mdf channel
f5e7632 Merge commit '647650de48d27934e0eb018db1c0ff4242ee39a5' into develop
647650d Merge pull request grpc-ecosystem#31 from tronprotocol/tx-timestamp
b290aad add hello message timestamp
6eba349 fix the transfer exception.
01a4166 Merge pull request grpc-ecosystem#30 from tronprotocol/update_account
80c5fd4 Merge branch 'master' into update_account
6cb5e8c Merge pull request grpc-ecosystem#528 from tronprotocol/refactor_transaction
7fc51a7 refactor the transaction structure
221cf70 Merge pull request grpc-ecosystem#525 from tronprotocol/refactor_transaction
7308a22 refactor the transaction structure
3868626 merge develop
a68e403 Merge commit 'd49019ec76c606ab1aa743a97710bac8b291cbf5' into add_grpc_api_for_solidity
d49019e Merge pull request grpc-ecosystem#28 from tronprotocol/fix_return
ff425d8 fix: return type.
1435bef Merge pull request grpc-ecosystem#27 from tronprotocol/add-reason-enum
2ec383f add ReasonCode
4d6bbda modify use LatestBlockHeaderNumber compare
d139597 modify use LatestBlockHeaderNumber compare
bc9ca33 merge protocol
8c6167e Merge pull request #25 from tronprotocol/xd-reason-code
130f5a2 modify reason code
6f41dfa Merge pull request #22 from tronprotocol/feature/add_db_api
9498448 feature: merge master to this.
13d4b62 Merge remote-tracking branch 'origin/master' into feature/add_db_api
246cf61 Merge pull request #24 from tronprotocol/add_database_api
2f7ae0b feature:add Datebase api Service.
a5e544c fix:change go_package.
c003d39 Merge remote-tracking branch 'origin/master' into feature/add_db_api
2dbee3b add WalletSolidity api service.
acc5c20 add get AssetIssueList by Timestamp api.
87c23fb add get Transaction api.
1033d54 Merge branch 'develop' into remove_header
c56402e add solidity node impl
f549760 add go package setting
7f1d31a add go package setting
0ef1de2 Merge branch 'develop' of https://github.com/tronprotocol/java-tron into develop
303dd5d feat: add BlockReference api
ed93332 Merge commit '1fab36b414599a3b24e739da4f04ee57bb4c14a5' into develop
d4a93ed improve: modified the seq num and fix build error
7acdfaf improve: add ref_block_num and ref_block_hash
c7f7d3e feat: remove utxo support from protocol.
59a3f02 Merge remote-tracking branch 'origin/develop' into develop
ac5eed9 add comments about ParticipateAssetIssueContract.
b1f5964 Merge commit 'ce4933fbe83c734ca244ecb0e8f8a9f854aa8afe' into protocol
336711d Merge commit '0297ff5d8a027576b0a7943cf0174ca7fc789534' into develop
f7d6d7b Merge commit 'ba4df0279d39dc07b6f6e40ccb0a66f3f5631e67' into feature/add_total_trans
0ed107a Merge commit '9f390c30504fe635e9d968393c2869f14cdfe9a4' into develop
4842483 Merge commit '06d60bdde8214ae47fecd8625e31dcd54c0bb324' into p2p
95b951b Merge branch 'develop' into p2p
192b1df Revert "Revert "Feature/devfor evan""
4e5ce06 Merge commit '9ac2d9b122b1e5ce429b4cc607741c56fbe27d4c' into p2p
fcc01fe Revert "Feature/devfor evan"
222e9e0 Merge remote-tracking branch 'origin/feature/devforEvan' into feature/devforEvan
0e407b9 merage proto
abb7a8b Merge commit '4668a8e92da61efdcb12d1bc7275d02373a7e052' into feature/devforEvan
e0dd5af Merge commit 'ca59ee900e2611553abebbf3a9f8feacab52228e' into feature/devforEvan
3f78d1b merge
94005a1 Merge commit 'ca59ee900e2611553abebbf3a9f8feacab52228e' into p2p
2836bdd change getBalance to getAccount
a97cfd7 Merge commit 'f1a1e24144dcefa5b604aa4a5ed69ef3bb934c66' into p2p
473587b merage proto
b74432d Merge remote-tracking branch 'origin/feature/devforEvan' into feature/devforEvan
312121f merage proto
faabdc4 Merge commit '044a39a4666b0428785d0501067bf417ade820c1' into p2p
82ceb8d Merge commit '13f1658d0497ecf4b6ce3ae9ed199b35cf654e21' into feature/devforEvan
f6ecbb5 Merge commit '0d9e0fdb313a8d4d5d3baaaea22569b72645fc08' into feature/devforEvan
8d0790f Merge commit 'bbeed13b2b052ae0fc669fb4a4ce1d5ac2cc9092' into p2p
8140964 Merge commit 'd08db0e07224a7aac3f296fc3a2e930e24d06fb5' into p2p
0aaff3d mdf discover msg
cf487bb Merge commit 'f63eda3db1b4386128cbc6235c3b95a72fbf0ba0' into p2p
dc9c1bb Merge commit '79c9b333f22367d3792f3faa2970b8dbbcf646b0' into feature/devforEvan
7abfb54 Merge commit '0f3ab59dfcba7e39789cdb810f3bd69a4fabb44b' into feature/devforEvan
5b0f8ac add getBlock
8901f2c add discover message
936b701 Merge commit '2f82ce89fcbebced73126885cf2feeda3463c73d' into develop
bbd0ea3 Merge commit '050740d1801ad990f2ad3c9eeee3030c74e022f3' into p2p
61decfb Merge commit '2795379f4fee8b12715e8835967f67925a5d68e7' into p2p
b5ced2d Merge branch 'develop' into junit/config
68b5a48 Merge pull request grpc-ecosystem#251 from sdargutev/develop
6b70dfd Merge pull request grpc-ecosystem#245 from tronprotocol/feature/updatewitness
935fd4c merged
c9410d8 refactoring
a3c7257 Merge remote-tracking branch 'origin/develop' into junit/config
9df3d33 Merge commit 'ec85aa1c2f62dbb6a027cd713e5965311dee29e6' into feature/updatewitness
fee98bf Merge remote-tracking branch 'origin/syncBlockChain' into syncBlockChain
7f4f302 rm duplicate instance
f509ce3 Merge branch 'develop' of https://github.com/tronprotocol/java-tron into syncBlockChain
774cceb Merge pull request grpc-ecosystem#237 from tronprotocol/miss_confuse_db
164d8bd remove dupicated number refresh
a58bf8e Merge branch 'develop' into junit/config
bb3a578 improve code readability
6910ef4 fix: add Account setter() validator

git-subtree-dir: protocol
git-subtree-split: e92f847
ithinker1991 pushed a commit to tronprotocol/grpc-gateway that referenced this issue Jun 18, 2018
…saction

refactor the transaction structure
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants