isFatal and asych

[golanha]

This change is 

[yehiyam]

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @golanha)

---

lib/core/client.js, line 67 at r1 (raw file):

    }

    isFatal(err) {

Add unittest

---

lib/layout/discovery/discovery.js, line 25 at r1 (raw file):

    }

    async set(option) {

Why do you need to add async here? and if you need, why not in delete, register, and all other methods.
Is it so you will know to await the result in the wrapper?
I think it will be better to wrap the methods in state-manager.js and not the low level methods.

[golanha]

Reviewable status: 1 of 6 files reviewed, 2 unresolved discussions (waiting on @yehiyam)

---

lib/core/client.js, line 67 at r1 (raw file):

Previously, yehiyam wrote…

Add unittest

Done.

---

lib/layout/discovery/discovery.js, line 25 at r1 (raw file):

Previously, yehiyam wrote…

Why do you need to add async here? and if you need, why not in delete, register, and all other methods.
Is it so you will know to await the result in the wrapper?
I think it will be better to wrap the methods in state-manager.js and not the low level methods.

Done.

[yehiyam]

Reviewed 4 of 5 files at r2, all commit messages.
Reviewable status: 5 of 6 files reviewed, 1 unresolved discussion (waiting on @golanha and @yehiyam)

---

tests/test.js, line 8 at r2 (raw file):

const triggersTreeExpected = require('./mocks/triggers-tree.json');
const Semaphore = require('await-done').semaphore;
const { ExecuteWrapper } = require('cockatiel/dist/common/Executor');

not used

Code quote:

 ExecuteWrapper

[yehiyam]

Reviewable status: 5 of 6 files reviewed, 2 unresolved discussions (waiting on @golanha and @yehiyam)

---

tests/test.js, line 2431 at r2 (raw file):

        describe('isFatal', () => {
            it('return isFatal true on etcd problem', async () => {
                const { GRPCGenericError, EtcdError } = require('etcd3');

why not require it at the top?

[yehiyam]

Reviewable status: 5 of 6 files reviewed, 3 unresolved discussions (waiting on @golanha and @yehiyam)

---

tests/test.js, line 2431 at r2 (raw file):

        describe('isFatal', () => {
            it('return isFatal true on etcd problem', async () => {
                const { GRPCGenericError, EtcdError } = require('etcd3');

required but not used

Code quote:

EtcdError

[golanha]

Reviewable status: 5 of 6 files reviewed, 3 unresolved discussions (waiting on @yehiyam)

---

tests/test.js, line 8 at r2 (raw file):

Previously, yehiyam wrote…

not used

Done.

---

tests/test.js, line 2431 at r2 (raw file):

Previously, yehiyam wrote…

why not require it at the top?

Done.

Code quote:

const { GRPCGenericError, EtcdError } = require('etcd3');

---

tests/test.js, line 2431 at r2 (raw file):

Previously, yehiyam wrote…

required but not used

Done.

[yehiyam]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @golanha)