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 #133

Merged
merged 3 commits 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

flyteorg/flyte#410

wild-endeavor
wild-endeavor previously approved these changes Jul 12, 2020
wild-endeavor
wild-endeavor previously approved these changes Jul 12, 2020
@codecov-commenter
Copy link

codecov-commenter commented Jul 12, 2020

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #133      +/-   ##
==========================================
- 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...0fa1f98. Read the comment docs.

@wild-endeavor wild-endeavor merged commit 3a2f521 into master Jul 12, 2020
@kumare3 kumare3 deleted the queuingbudge-bugfix branch July 12, 2020 22:06
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