This repository has been archived by the owner on Feb 22, 2023. It is now read-only.
modify client to write logs #476
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jkelvie
suggested changes
Aug 13, 2018
bin/bst-speak.ts
Outdated
| @@ -63,6 +64,7 @@ Global.initializeCLI().then( | |||
| console.log("Your token is saved, you can now use this command without providing a token"); | |||
| } | |||
|
|
|||
| BstStatistics.instance().record(BstCommand.speak); | |||
There was a problem hiding this comment.
Why do we not capture the node ID? We should always try to record it, but if it has not been setup, then do not send it.
bin/bst-test.ts
Outdated
| @@ -13,5 +14,6 @@ program | |||
|
|
|||
| const testCLI = new skillTesting.CLI(); | |||
| testCLI.run(process.argv).then((success) => { | |||
| BstStatistics.instance().record(BstCommand.test); | |||
There was a problem hiding this comment.
Same as above - we should include the nodeID if it it has been created.
ecruzado
force-pushed
the
modifyClientToWriteLogs
branch
from
ae83987
to
b2a87db
Compare
jkelvie
suggested changes
Aug 14, 2018
bin/bst-test.ts
Outdated
| .usage("[test-pattern-regex]") | ||
| .description("Runs unit-tests for a skill - automatically searches for YML test files and runs them") | ||
| .parse(process.argv); | ||
| Global.initializeCLI().then(() => { |
There was a problem hiding this comment.
Can we add an optional parameter to this, which does not generate an ID if there is not already one?
This is because we are commonly going to use this command with CI/CD tools, where they will constantly be re-creating this. We do not want to continuously generate new IDs.
There should be a new flag, bootstrap, that gets passed to this method:
https://github.com/bespoken/bst/blob/master/lib/client/bst-config.ts#L23
If it is set to false, then it will not create a new ID (i.e., it will not call the bootstrapIfNeeded method).
By default, it should be set to true, as that is typically what we want.
There was a problem hiding this comment.
that is the current behavior, an ID is only created if the file "~/.bst/config" is missing, otherwise we use the id that is on the file
jkelvie
approved these changes
Aug 14, 2018
ecruzado
force-pushed
the
modifyClientToWriteLogs
branch
from
a1e25d0
to
c612beb
Compare
jperata
added a commit
that referenced
this pull request
Sep 5, 2018
* Initial work to integrate skill-tester with bst * Updated generated API docs * 2.0.0-0 * Updated generated API docs * 3.0.0-0 * Updated generated API docs * 2.0.0-1 * Updated test command * Updated generated API docs * 2.0.0-2 * Updates for new skill-tester version * Updated generated API docs * 2.0.0-3 * Added options to skip initializing a new config * Updated generated API docs * 2.0.0-4 * Updated skill testing version Added process.exit codes for success or failure * Updated generated API docs * 2.0.0 * Updated skill-testing version * Updated generated API docs * 2.0.1 * New skill-testing version * Updated generated API docs * 2.0.2 * Fixed issue with debug output not showing up due to process.exit call * Updated generated API docs * 2.0.3 * Updated generated API docs * 2.0.4 * Version2.0.5 (#459) * update skill tester * Updated generated API docs * 2.0.5 * Version2.0.7 (#461) * update skill-testing-ml * Updated generated API docs * 2.0.7 * Version2.0.8 (#464) * update skill tester * Updated generated API docs * 2.0.8 * Version 2.0.9 (#466) * update skill tester package * Updated generated API docs * 2.0.9 * remove deploy command (#471) * remove deploy command * skip test * Modify BST Server to write logs #469 (#475) * Modify BST Server to write logs * modify client to write logs (#476) * modify client to write logs * include node id on speak and test commands * dont create config file on test command * remove default paramenters to support node 4 * skip statistics on CI * add env var for source api url * override configuration from cli (#477) * override configuration from cli * add test * update virtual device sdk (#479) * Warn on no using last version (#486) * warn user on not using last version * display custom messages * order options on test, fix default parameter (#488) * order options on test, default parameter * remove array options * update skill tester package, order options by name * Updated generated API docs * 2.0.17 * Version2 0 20 (#492) * Skill testing update * Updated generated API docs * 2.0.20 * Migrate to Circle 2.0 (#494) * Migrate to Circle 2.0 * Include also bst-test branch to circle allowed branches * Remove invalid identation * Change from multiline to single line (#495) * Regenerate docs
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.