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

API and client improvements and fixes #885

Merged
merged 24 commits into from
Nov 9, 2018

Conversation

peterdemartini
Copy link
Contributor

@peterdemartini peterdemartini commented Nov 6, 2018

  • Add cluster to prometheus labels in /cluster/stats
  • Add the ability to filter executions by multiple statuses
  • Add the ability to get all execution errors to API and errors
  • Fix issue jobs vs list issue in teraslice-client-js
  • Update API docs
  • Make API endpoints consistent
  • Changed default search limit from 10000 to 100.
  • Update a few dependencies

Resolves #879
Resolves #884
Resolves #880
Resolves #877
Resolves #644

@codecov
Copy link

codecov bot commented Nov 6, 2018

Codecov Report

Merging #885 into master will increase coverage by 1.04%.
The diff coverage is 18.08%.

@@            Coverage Diff            @@
##           master    #885      +/-   ##
=========================================
+ Coverage   58.36%   59.4%   +1.04%     
=========================================
  Files         203     203              
  Lines        9054    9036      -18     
  Branches      254     254              
=========================================
+ Hits         5284    5368      +84     
+ Misses       3688    3587     -101     
+ Partials       82      81       -1
Impacted Files Coverage Δ
packages/teraslice-client-js/lib/jobs.js 100% <ø> (+9.09%) ⬆️
packages/teraslice/lib/cluster/services/jobs.js 0% <0%> (ø) ⬆️
...ckages/teraslice/lib/cluster/services/execution.js 0% <0%> (ø) ⬆️
packages/teraslice/lib/cluster/services/assets.js 0% <0%> (ø) ⬆️
packages/teraslice-cli/cmds/jobs/lib/index.js 0% <0%> (ø) ⬆️
packages/teraslice-client-js/lib/ex.js 60.6% <100%> (+12.6%) ⬆️
packages/teraslice/lib/cluster/services/api.js 23.14% <15.06%> (+23.14%) ⬆️
packages/teraslice/lib/utils/api_utils.js 50.81% <58.82%> (+18.38%) ⬆️
...s/job-components/src/operations/parallel-slicer.ts 92.1% <0%> (+2.63%) ⬆️

@peterdemartini peterdemartini changed the title API and client-js improvements and fixes API and client improvements and fixes Nov 6, 2018
@peterdemartini peterdemartini changed the title API and client improvements and fixes WIP: API and client improvements and fixes Nov 6, 2018
@peterdemartini peterdemartini force-pushed the api-and-client-improvements branch from b80498c to 6016a58 Compare November 8, 2018 13:17
@peterdemartini peterdemartini force-pushed the api-and-client-improvements branch from 112f191 to 3fd8d2d Compare November 8, 2018 21:20
@peterdemartini peterdemartini changed the title WIP: API and client improvements and fixes API and client improvements and fixes Nov 8, 2018
@peterdemartini
Copy link
Contributor Author

I am removing WIP, I think it is ready to go, but it should be reviewed by testing locally

@kstaken
Copy link
Member

kstaken commented Nov 9, 2018

Is the N/A here intentional?

curl  localhost:5678/txt/controllers
name            job_id                                workers_available  workers_active  failed  queued  processed
--------------  ------------------------------------  -----------------  --------------  ------  ------  ---------
Data Generator  7ab049eb-0a0f-42ff-be5c-cf979c2b0f5c  1                  N/A             N/A     1       57

@peterdemartini
Copy link
Contributor Author

@kstaken yes, and no, instead of having a blank value, we put 'N/A' to indicate that this value doesn't exist since applying default values. But your example should show zeros, I will fix that

@godber
Copy link
Member

godber commented Nov 9, 2018

@kstaken since you have it up and running can you share the output of

curl -Ss -H "Accept: blah application/openmetrics-text; blah" localhost:5678/cluster/stats

here?

@kstaken
Copy link
Member

kstaken commented Nov 9, 2018

# TYPE teraslice_slices_processed counter
teraslice_slices_processed{cluster="teracluster2"} 963
# TYPE teraslice_slices_failed counter
teraslice_slices_failed{cluster="teracluster2"} 0
# TYPE teraslice_slices_queued counter
teraslice_slices_queued{cluster="teracluster2"} 2
# TYPE teraslice_workers_joined counter
teraslice_workers_joined{cluster="teracluster2"} 2
# TYPE teraslice_workers_disconnected counter
teraslice_workers_disconnected{cluster="teracluster2"} 0
# TYPE teraslice_workers_reconnected counter
teraslice_workers_reconnected{cluster="teracluster2"} 0

@godber
Copy link
Member

godber commented Nov 9, 2018

Looks good, thanks @kstaken

@kstaken
Copy link
Member

kstaken commented Nov 9, 2018

Stopping on ex seemed to work but using jobs does this and does not stop.

curl -XPOST localhost:5678/jobs/c9fbc8ec-8d21-4f03-936d-53e72c18b505/_stop
{
    "status": "terminated"
}

@kstaken
Copy link
Member

kstaken commented Nov 9, 2018

That is reporting correct however the job_id I was using was retrieved using /txt/jobs?size=1 which gave me an old job. Appears the default sorting has been lost.

@kstaken
Copy link
Member

kstaken commented Nov 9, 2018

Using the correct job_id stop works fine but /txt/jobs should give you newest jobs first.

Copy link
Member

@kstaken kstaken left a comment

Choose a reason for hiding this comment

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

This seems good.

@peterdemartini peterdemartini merged commit 6b228e7 into master Nov 9, 2018
@peterdemartini peterdemartini deleted the api-and-client-improvements branch November 9, 2018 20:56
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.

3 participants