67P / kredits-web

Kredits Web UI
https://kredits.kosmos.org
6 stars 2 forks source link

Use new healthcheck API from kredits-contracts #59

Closed bumi closed 5 years ago

bumi commented 6 years ago

This uses console.error to log the healthcheck error.

bumi commented 6 years ago

what do you think? merge it?

raucao commented 6 years ago

Don't know what it does. Could you link to the documentation for that healthcheck function?

bumi commented 6 years ago

It checks if IPFS is available and if the contracts are found. https://github.com/67P/kredits-contracts/blob/master/lib/utils/healthcheck.js#L6

If not it logs an error. this is helpful as it might happen in dev mode some times (as it did) and then only causes strange error messages that are not clear and not directly obvious.

raucao commented 6 years ago

Hmm, I expected something very different, more like if we have a live connection to the Ethereum node or something. How about we rename it in some way that is more expressive about the setup being correct? Like e.g. "checkRequirements" or something. Naming is hard, of course. Can't think of anything ideal just now.

bumi commented 6 years ago

connection is checked... as it loads the code of the address through the ethereum node. I don't care that much about the naming, just wanted to have something general as it contains a few checks and can be extended.

raucao commented 6 years ago

How about "preflight" or "preflightChecks"?

This is what the process is called in an aircraft, when the pilot checks that everything is ok, before taking off. It's also what they call the CORS OPTIONS requests that ensure one can e.g. POST a thing across JS origins.

bumi commented 6 years ago

@skddc renamed the function call. what do you think?

raucao commented 5 years ago

@bumi Is this one still relevant?

bumi commented 5 years ago

yes, preflightChecks tests if and IPFS connection is available and if all the contracts are found. The later is probably not so important anymore because if we have an address of a contract the contract is probably also deployed. But I think it is still good to check as one of the main issue has always been the many moving parts that could break.

so tl;dr: yes, I'd still merge it.

raucao commented 5 years ago

Ok, want to resolve the merge conflicts in this case?

bumi commented 5 years ago

closing this and opening a new one.