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

bug; Queuing budget no longer exists. Quality of service is not yet integrated #135

Merged
merged 1 commit into from
Jul 12, 2020

Conversation

kumare3
Copy link
Contributor

@kumare3 kumare3 commented Jul 12, 2020

TL;DR

This fix removes access of queuing_budget in flytekit as this is not used and there has been refactoring that renders the current Interface incorrect. Problems,
queuing_budget renamed to queueing_budget
field 1 in WorkflowMetadata is now QualityOfService instead
queueing budget is part of the QOSSpec and is one of.
Usage for the users was still access to queueing budget

Problems:

Flytekit was still accessing workflowmetadata.queuing_budget and this breaks in production for anyone who upgrades to latest flytekit.

Type

Bug Fix
Feature
Plugin

Are all requirements met?

Code completed
Smoke tested
Unit tests added
Code documentation added
Any pending items have an associated Issue
Complete description
Currently eliminated queuing_budget as it is not used. It should be re-introduced carefully thinking about the interface

Tracking Issue

flyteorg/flyte#410

Follow-up issue

NA

@codecov-commenter
Copy link

codecov-commenter commented Jul 12, 2020

Codecov Report

Merging #135 into master will decrease coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #135      +/-   ##
==========================================
- Coverage   81.38%   81.35%   -0.04%     
==========================================
  Files         217      217              
  Lines       14153    14124      -29     
  Branches     1160     1158       -2     
==========================================
- Hits        11518    11490      -28     
+ Misses       2353     2352       -1     
  Partials      282      282              
Impacted Files Coverage Δ
tests/flytekit/unit/sdk/test_workflow.py 96.66% <ø> (+0.54%) ⬆️
flytekit/__init__.py 100.00% <100.00%> (ø)
flytekit/common/workflow.py 74.85% <100.00%> (ø)
flytekit/models/core/workflow.py 100.00% <100.00%> (ø)
flytekit/sdk/workflow.py 100.00% <100.00%> (ø)
tests/flytekit/unit/models/core/test_workflow.py 100.00% <100.00%> (ø)

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 ea9970b...e1ba68a. Read the comment docs.

@kumare3 kumare3 merged commit 4db091f into master Jul 12, 2020
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