Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

python-client: Improve error checks, update readme, add documentation #1010

Merged
merged 4 commits into from
Apr 2, 2019

Conversation

samhattangady
Copy link
Contributor

ConductorWorker.py was only checking if worker response was not None.
So if the worker was responding with a dict, the error raised would be
much harder to understand.

For example, a worker that was giving a response without logs key in
the dict showed an error

Error executing task: 'logs'

This MR aims to fix that. Also have updated the kitchensink example in
the README.md to return the logs field, and some typo fixes.

`ConductorWorker.py` was only checking if worker response was not None.
So if the worker was responding with a dict, the error raised would be
much harder to understand.

For example, a worker that was giving a response without `logs` key in
the dict showed an error
> Error executing task: 'logs'

This MR aims to fix that. Also have updated the kitchensink example in
the `README.md` to return the `logs` field, and some typo fixes.
@samhattangady
Copy link
Contributor Author

samhattangady commented Feb 25, 2019

Please let me know if logs is supposed to be an optional field. I can do that as well. I wasn't able to figure it out from the docs.

The schema on API docs page doesn't mention logs.

@codecov
Copy link

codecov bot commented Feb 25, 2019

Codecov Report

Merging #1010 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##                dev    #1010   +/-   ##
=========================================
  Coverage     62.68%   62.68%           
  Complexity     2605     2605           
=========================================
  Files           231      231           
  Lines         13095    13095           
  Branches       1310     1310           
=========================================
  Hits           8208     8208           
  Misses         4160     4160           
  Partials        727      727

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b0647c...b55521a. Read the comment docs.

@coveralls
Copy link

coveralls commented Feb 25, 2019

Pull Request Test Coverage Report for Build 2466

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 68.716%

Totals Coverage Status
Change from base Build 2465: 0.0%
Covered Lines: 9208
Relevant Lines: 13400

💛 - Coveralls

add logs to execute_4, and add parens for print statement
@samhattangady
Copy link
Contributor Author

I am not sure why the tests are failing. The second commit only changed the client/python/README.md file. Please let me know what should be done to fix this.

@samhattangady samhattangady changed the title python-client: Improve error checks, update readme. python-client: Improve error checks, update readme, add documentation Mar 11, 2019
@kishorebanala kishorebanala merged commit 6dc08cb into Netflix:dev Apr 2, 2019
long-64 pushed a commit to long-64/conductor that referenced this pull request Oct 2, 2019
…ovements

python-client: Improve error checks, update readme, add documentation
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants