Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed serveral bugs from #874 #888

Merged
merged 8 commits into from
Nov 21, 2018

Conversation

jeffmontagna
Copy link
Contributor

  • cluster_sh shouldn't allow files only object_ids.
    • Removed option to have files in cluster_sh.
    • Added more error checking for invalid entries
  • assets
    • list -- this command doesn't exist but should. Added command.
    • deploy -- does not appear to add the meta data to assets.json. Updated to now update asset.json
  • ex errors
  • jobs
    • Commands that take .json files should not require the file extension. Removed .json requirement.
    • init command not implemented. Added command
    • restart cluster --all error message when stopping prevents starting of jobs
    • start error after starting job, Cannot read property 'workers_active' of undefined
    • Fixed issue with restart in which the stop loop would exit prematurely and try to start up jobs. Replaced for loops with a map which resulted in faster job restarting because and removed the premature exit.

@jeffmontagna jeffmontagna requested a review from godber November 7, 2018 21:35
@codecov
Copy link

codecov bot commented Nov 7, 2018

Codecov Report

Merging #888 into master will decrease coverage by 1.51%.
The diff coverage is 5.89%.

@@            Coverage Diff             @@
##           master     #888      +/-   ##
==========================================
- Coverage   59.19%   57.68%   -1.52%     
==========================================
  Files         210      223      +13     
  Lines        9222     9410     +188     
  Branches      278      278              
==========================================
- Hits         5459     5428      -31     
- Misses       3676     3895     +219     
  Partials       87       87
Impacted Files Coverage Δ
packages/teraslice-cli/index.js 0% <ø> (ø) ⬆️
packages/teraslice-cli/cmds/jobs/lib/cli.js 0% <ø> (ø)
packages/teraslice-cli/cmds/tjm/index.js 0% <0%> (ø)
packages/teraslice-cli/cmds/jobs/status.js 0% <0%> (ø) ⬆️
packages/teraslice-cli/cmds/lib/annotation.js 0% <0%> (ø) ⬆️
packages/teraslice-cli/cmds/lib/asset.js 0% <0%> (ø) ⬆️
packages/teraslice-cli/cmds/aliases/update.js 0% <0%> (ø) ⬆️
packages/teraslice-cli/cmds/tjm/lib/index.js 0% <0%> (ø)
packages/teraslice-cli/cmds/jobs/list.js 0% <0%> (ø) ⬆️
packages/teraslice-cli/cmds/tjm/errors.js 0% <0%> (ø)
... and 73 more

@kstaken
Copy link
Member

kstaken commented Nov 8, 2018

In the examples there is this which doesn't work. Not sure if it is supposed to.

earl jobs list http://cluster1.net:80

@terascope terascope deleted a comment Nov 16, 2018
@terascope terascope deleted a comment Nov 16, 2018
@terascope terascope deleted a comment Nov 16, 2018
@terascope terascope deleted a comment Nov 16, 2018
@terascope terascope deleted a comment Nov 16, 2018
@terascope terascope deleted a comment Nov 16, 2018
@terascope terascope deleted a comment Nov 16, 2018
@peterdemartini
Copy link
Contributor

I recommend you rebase or merge master in

Copy link
Member

@godber godber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jeff and I went through some of this. We'll merge this for now and then work on refactoring and fixes in the future. We'll probably wait to bump the version until my changes and perhaps some improvements are made.

@godber godber merged commit 66eefb0 into terascope:master Nov 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants