societal grant 2 milestone 2 submission#806
Conversation
*added Grant 2 Milestone 2 Delivery
|
@gfox1 thank you for the milestone submission. Please see the evaluation document and provide proper answers and fixes. After that, let me know to continue with the evaluation. |
Using `release/democracy` branch for evaluation.
*updated societal-node github repo links
Hi @dsm-w3f ! Thanks for the evaluation. Here are the comments regarding the evaluation document: Docker and System TestRunning the instance locally is quite a big deal right now since we're using a node-indexer middleware that serves as an additional GraphQL API for our client and storing some off-chain data that is required for the Client Application. Here is the link to GitHub repo for it: https://github.com/sctllabs/node-indexer. There are 4 services running as a part of the node-indexer so you'll have to spin up 6 services overall locally to make the whole application work:
Thats's why we provided the link to our dev instance to showcase the milestone delivery seamlessly for the evaluator. In case if you need a guide to spin up a local instance of the node-indexer - we can provide it. However as I've already mentioned it is a bit tricky to spin up all the things locally. Code QualityI've updated link to the societal-node repo to our current P.S. As for the UI issue with 'Approve Origin` - which browser have you used? Seems like it's working fine for us in Chrome. |
|
@okalenyk thank you for the answer. The matter here is that we are working with open-source software that should be reusable for other developers. If the application only works with these other services, you should provide a way for developers to start all these services locally in order to enable them to develop something using your code as a base if needed. This is the reason why I asked for local installation instructions. Ideally, a docker-compose file that spins up all the services locally and lets the application ready-to-use should be a good solution to fulfill this requirement. |
|
@dsm-w3f Thanks for the clarification. We've updated the submission PR changing the following sections:
We've also gathered all the info required for running the application locally there in the docker-compose-full.yml file. Please refer to Run in Docker section of the Readme file there. Please let us know if you have any questions there. |
*using node-indexer docker-compose-full.yml as a reference
|
Hey @dsm-w3f, I just wanted to follow up to see if our changes worked for you? |
|
@gfox1 I tried again using the docker compose provided but without success. Please see the details in the evaluation document. Please fix the problems reported and let me know when I can continue this evaluation. |
|
Hi @dsm-w3f! Thanks for the detailed updates! Environment Launch: I've updated launch script as well as the image references inside docker-compose-full.yml file. Please make sure you pull latest images for societal-node and node-indexer to make things work. I've also tested the launch script with clean clone of the repo and looks like it works now without additional efforts. Unit Test Well, the main branch is ahead of the democracy release so definitely it has more tests than this particular one that is used for the submission. As for this branch - I haven't noticed any tests that were filtered out from the eventual batch. Code Quality I've tried to fix as many warnings as possible but most of them coming from the weights files that are auto-generated from benchmark framework. Anyway, I managed to get rid of the warnings from there too. As for the warning Very complex type used... - in most cases it is coming from usage of the storage double map and we're using it to persist data per each DAO so in fact there is nothing we could do there. Regarding warnings from dao-assets pallet - we're using the code coming from the parity substrate main repo adding functionality that we consider is missing in the main frame pallet for reservable/lockable assets and unit tests for that functionality as well. That's why we didn't make any changes coming upstream from the main repo trying to isolate the and focus on the changes required for making assets reservable/lockable - it is a part of the milestone 1 submission of the grant. |
|
@okalenyk thank you for the answer and fixes. I'm having a small problem with docker but after a workaround, the application worked. Please see the evaluation document for details. We are almost finishing this evaluation. Just this small fix remains. |
@dsm-w3f thanks for the evaluation! It looks like there was a problem either with the docker hub itself or an intermittent internet connectivity problems while pulling new images from the hub. I don't see any problems right now while pulling images and the registries provided are public as well. The link that is provided as a part of the error: https://registry-1.docker.io/v2/societal/node-indexer/manifests/release-democracy-latest is not accessible for all the public images hosted on the docker hub. For instance - a Parity tech polkadot image: https://registry-1.docker.io/v2/parity/polkadot/manifests/latest - this link is also return forbidden error. So I think it was a temporary issue with the docker hub or a temporary connectivity issue that happened while trying to pull images for docker-compose. |
|
@okalenyk thank you for the answer. The docker problem was solved. The milestone is approved. I'll forward your invoice and the payment should take place within two weeks. Great work! |
|
We noticed that this is the last milestone of your project. Congratulations on completing your grant! 🎊 |
|
Hey @dsm-w3f, thanks for reviewing this so quickly and approving the grant. Cheers! |
|
hi @gfox1 we transferred the payment today. |
|
Hi @RouvenP, thank you. We have received the payment. |
Milestone Delivery Checklist
Link to the application pull request: w3f/Grants-Program#1450