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

Added default no limit on storage #93

Merged
merged 3 commits into from
Jul 26, 2021
Merged

Added default no limit on storage #93

merged 3 commits into from
Jul 26, 2021

Conversation

yindia
Copy link
Contributor

@yindia yindia commented Jul 17, 2021

TL;DR

Context :- A user started discussion regarding a bug in copiliot. where copiliot failes with large files because of stdlib limit check. We don't want any limit on size in case of copiliot

  • If GetLimitMegabytes is 0 then don't check size limit

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

How did you fix the bug, make the feature etc. Link to any design docs etc

Tracking Issue

Remove the 'fixes' keyword if there will be multiple PRs to fix the linked issue

fixes flyteorg/flyte#1251
flyteorg/flyte#1253
flyteorg/flyte#1287

Follow-up issue

NA
OR
https://github.com/flyteorg/flyte/issues/

@codecov
Copy link

codecov bot commented Jul 17, 2021

Codecov Report

Merging #93 (ef929ab) into master (92b25c6) will increase coverage by 1.38%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #93      +/-   ##
==========================================
+ Coverage   65.49%   66.87%   +1.38%     
==========================================
  Files          59       59              
  Lines        2275     2817     +542     
==========================================
+ Hits         1490     1884     +394     
- Misses        642      788     +146     
- Partials      143      145       +2     
Flag Coverage Δ
unittests 65.28% <100.00%> (-0.21%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
storage/stow_store.go 73.01% <100.00%> (+1.75%) ⬆️
logger/logger.go 54.26% <0.00%> (-8.60%) ⬇️
storage/config_flags.go 53.33% <0.00%> (-2.23%) ⬇️
config/viper/viper.go 6.86% <0.00%> (-1.16%) ⬇️
atomic/atomic.go 73.91% <0.00%> (-1.09%) ⬇️
sets/generic_set.go 94.56% <0.00%> (-0.44%) ⬇️
config/url.go 100.00% <0.00%> (ø)
errors/errors.go 100.00% <0.00%> (ø)
ioutils/bytes.go 100.00% <0.00%> (ø)
futures/future.go 100.00% <0.00%> (ø)
... and 49 more

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 92b25c6...ef929ab. Read the comment docs.

@EngHabu
Copy link
Contributor

EngHabu commented Jul 17, 2021

Can you elaborate on why this change?

storage/config.go Outdated Show resolved Hide resolved
storage/stow_store.go Outdated Show resolved Hide resolved
Signed-off-by: Yuvraj <[email protected]>
@yindia yindia requested a review from kumare3 July 20, 2021 15:26
Signed-off-by: Yuvraj <[email protected]>
@yindia yindia force-pushed the fix/storage-limit branch from cca9567 to ef929ab Compare July 26, 2021 05:53
@yindia yindia merged commit 9f637fd into master Jul 26, 2021
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
* Added default no limit on storage

Signed-off-by: Yuvraj <[email protected]>
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.

[Housekeeping] Copilot should be allowed to download unlimited data - if needed
4 participants