kredits/kredits-web
9
0
Fork 0
Code Issues 21 Pull Requests Actions Releases 19 Activity

Use new healthcheck API from kredits-contracts #59

Closed
bumi wants to merge 4 commits from refactor/healthcheck into master
pull from: refactor/healthcheck
merge into: :master
Conversation 12 Commits 4 Files Changed 2
+5 -1
bumi commented 2018-04-21 17:10:58 +00:00 (Migrated from github.com)
Copy Link
Copy Source

This uses console.error to log the healthcheck error.

This uses console.error to log the healthcheck error.
fsmanuel (Migrated from github.com) reviewed 2018-04-21 17:10:58 +00:00
bumi commented 2018-05-03 15:12:31 +00:00 (Migrated from github.com)
Copy Link
Copy Source

what do you think? merge it?

what do you think? merge it?
raucao commented 2018-05-03 15:17:02 +00:00 (Migrated from github.com)
Copy Link
Copy Source

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

Don't know what it does. Could you link to the documentation for that healthcheck function?
bumi commented 2018-05-03 18:37:32 +00:00 (Migrated from github.com)
Copy Link
Copy Source

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.

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 2018-05-03 19:28:15 +00:00 (Migrated from github.com)
Copy Link
Copy Source

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.

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 2018-05-03 19:51:36 +00:00 (Migrated from github.com)
Copy Link
Copy Source

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.

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 2018-05-03 19:55:53 +00:00 (Migrated from github.com)
Copy Link
Copy Source

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.

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 2018-06-07 13:41:50 +00:00 (Migrated from github.com)
Copy Link
Copy Source

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

@skddc renamed the function call. what do you think?
raucao commented 2019-03-29 12:06:43 +00:00 (Migrated from github.com)
Copy Link
Copy Source

@bumi Is this one still relevant?

@bumi Is this one still relevant?
bumi commented 2019-03-29 12:33:31 +00:00 (Migrated from github.com)
Copy Link
Copy Source

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.

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 2019-03-29 12:55:28 +00:00 (Migrated from github.com)
Copy Link
Copy Source

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

Ok, want to resolve the merge conflicts in this case?
👍 1
bumi commented 2019-04-03 18:08:04 +00:00 (Migrated from github.com)
Copy Link
Copy Source

closing this and opening a new one.

closing this and opening a new one.

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
fsmanuel
Labels
Clear labels
bug
Something is not working
 dev environment
Config, builds, CI, deployment, etc.
 docs
Documentation
 duplicate
This issue or pull request already exists
 enhancement
Improving existing functionality
 feature
New functionality
 idea
Something to consider
 invalid
Not a bug
 kredits-1
Small contribution
 kredits-2
Medium contribution
 kredits-3
Large contribution
 question
Looking for an answer
release
major
release
minor
release
patch
security
All your base are belong to us
 ui/ux
User interface, process design, etc.
 wontfix
This won't be fixed
No labels
Milestone
No items
No Milestone
Assignees
Clear assignees
bkero (bkero) bumi (bumi) fsmanuel (fsmanuel) galfert (galfert) greg (Greg) haythem (haythem) hueso (hueso) raucao (Râu Cao) slvrbckt (Nick Jennings)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kredits/kredits-web#59