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.

Sign up for GitHub

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

Jump to bottom

[BUG] If a task only specifies resource limit, admin sets the request to a value potentially lower than the limit #264

Closed
3 of 20 tasks
EngHabu opened this issue Apr 13, 2020 · 1 comment
Closed
3 of 20 tasks

[BUG] If a task only specifies resource limit, admin sets the request to a value potentially lower than the limit #264

EngHabu opened this issue Apr 13, 2020 · 1 comment
Assignees
@katrogan
Labels
bug Something isn't working

Comments

@EngHabu
Copy link
Contributor

EngHabu commented Apr 13, 2020

Describe the bug
If a task sets memory_limit=200Mi, admin will set the memory_request=500Mi since that's the default it's configured with. This creates an invalid spec. Limit has to be >= request.

Expected behavior
Admin should set request == limit if limit is set, or to the default if not... and should do the same for the limit (set it == request if not set or to the default limit if not set)

Flyte component

  • Overall
  • Flyte Setup and Installation scripts
  • Flyte Documentation
  • Flyte communication (slack/email etc)
  • FlytePropeller
  • FlyteIDL (Flyte specification language)
  • Flytekit (Python SDK)
  • FlyteAdmin (Control Plane service)
  • FlytePlugins
  • DataCatalog
  • FlyteStdlib (common libraries)
  • FlyteConsole (UI)
  • Other

Environment
Flyte component

  • Sandbox (local or on one machine)
  • Cloud hosted
    • AWS
    • GCP
    • Azure
  • Baremetal
  • Other

Additional context
https://github.com/lyft/flyteadmin/blob/2a065109c8b14f560ef27aa74df67b157c52d9d1/pkg/manager/impl/execution_manager.go#L301-L303
@EngHabu EngHabu added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels Apr 13, 2020
@bnsblue
Copy link
Contributor

bnsblue commented Apr 13, 2020

@EngHabu Thank you for filing this bug

@katrogan katrogan mentioned this issue May 21, 2020
Closed
8 tasks
@katrogan katrogan mentioned this issue Sep 21, 2020
Merged
8 tasks
@katrogan katrogan removed the untriaged This issues has not yet been looked at by the Maintainers label Sep 22, 2020
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 6, 2022
@EngHabu @kumare3
Change branch compilation to avoid adding downstream deps (flyteorg#264)
f06e98d 
* Change branch compilation to avoid adding downstream deps

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Fix deepcopy generated bug

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Add checks to make sure compiled workflows remain the same

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Additional unit test to test branch spec generation

Signed-off-by: Ketan Umare <[email protected]>

* Fix compiler unit tests

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Fixed unit test

Signed-off-by: Ketan Umare <[email protected]>

* Fix one unit test and shorten json tags for connections

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Use GetConnections() instead of DeprecatedConnections

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Put back branch node upstream check in CanExecute predicate for backward compatibility

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* lint

Signed-off-by: Haytham Abuelfutuh <[email protected]>

Co-authored-by: Ketan Umare <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 6, 2022
Prefer to set task limits from requests when only the latter is set (f…
cdc8f67 
…lyteorg#264)
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 20, 2022
@SandraGH5
Update simple.py (flyteorg#264)
ad5348e
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 20, 2022
@Pianist038801
fix: recent workflow bug on filtering executions (flyteorg#264) #patch
0e82355 
Signed-off-by: Pianist038801 <[email protected]>

Co-authored-by: Pianist038801 <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 20, 2022
@flyte-bot
chore(release): Release 0.39.1 [skip ci]
ca2e7ec 
## [0.39.1](http://github.com/lyft/flyteconsole/compare/v0.39.0...v0.39.1) (2022-01-14)

### Bug Fixes

* recent workflow bug on filtering executions ([flyteorg#264](http://github.com/lyft/flyteconsole/issues/264)) #patch ([0e82355](http://github.com/lyft/flyteconsole/commit/0e823553bca31d0882c767e12235601e359a6475))
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 20, 2022
@pmahindrakar-oss
Added endpoint suffix to service user for pkce token saved in token c…
85a80af 
…ache (flyteorg#264)

* Added endpoint suffix to service user for pkce token saved in token cache

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* nits

Signed-off-by: Prafulla Mahindrakar <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Aug 9, 2023
@EngHabu @kumare3
Change branch compilation to avoid adding downstream deps (flyteorg#264)
89e74aa 
* Change branch compilation to avoid adding downstream deps

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Fix deepcopy generated bug

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Add checks to make sure compiled workflows remain the same

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Additional unit test to test branch spec generation

Signed-off-by: Ketan Umare <[email protected]>

* Fix compiler unit tests

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Fixed unit test

Signed-off-by: Ketan Umare <[email protected]>

* Fix one unit test and shorten json tags for connections

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Use GetConnections() instead of DeprecatedConnections

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Put back branch node upstream check in CanExecute predicate for backward compatibility

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* lint

Signed-off-by: Haytham Abuelfutuh <[email protected]>

Co-authored-by: Ketan Umare <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Aug 21, 2023
Prefer to set task limits from requests when only the latter is set (f…
23c5685 
…lyteorg#264)
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Apr 30, 2024
@pmahindrakar-oss
Added endpoint suffix to service user for pkce token saved in token c…
e6855b4 
…ache (flyteorg#264)

* Added endpoint suffix to service user for pkce token saved in token cache

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* nits

Signed-off-by: Prafulla Mahindrakar <[email protected]>
austin362667 pushed a commit to austin362667/flyte that referenced this issue May 7, 2024
@pmahindrakar-oss @austin362667
Added endpoint suffix to service user for pkce token saved in token c…
ab1371d 
…ache (flyteorg#264)

* Added endpoint suffix to service user for pkce token saved in token cache

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* nits

Signed-off-by: Prafulla Mahindrakar <[email protected]>
robert-ulbrich-mercedes-benz pushed a commit to robert-ulbrich-mercedes-benz/flyte that referenced this issue Jul 2, 2024
@pmahindrakar-oss @robert-ulbrich-mercedes-benz
Added endpoint suffix to service user for pkce token saved in token c…
ab83aa5 
…ache (flyteorg#264)

* Added endpoint suffix to service user for pkce token saved in token cache

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* nits

Signed-off-by: Prafulla Mahindrakar <[email protected]>
troychiu pushed a commit that referenced this issue Jul 8, 2024
@andrewwdye
Add golangci config (#264)
e1f366c 
## Overview
Adds a golangci config (matching unionai/cloud) and does a bunch of clean up to get things _mostly_ lint free

## Test Plan
Ran a sanity test with actors locally, but otherwise should be low risk (just refactoring)

## Rollout Plan (if applicable)
N/A

## Upstream Changes
Should this change be upstreamed to OSS (flyteorg/flyte)? If so, please check this box for auditing. Note, this is the responsibility of each developer. See [this guide](https://unionai.atlassian.net/wiki/spaces/ENG/pages/447610883/Flyte+-+Union+Cloud+Development+Runbook/#When-are-versions-updated%3F).
- [ ] To be upstreamed

## Issue
fixes EXO-100
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@katrogan katrogan

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@EngHabu @katrogan @bnsblue