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

Fix read raw limit #4370

Merged
merged 1 commit into from
Nov 6, 2023
Merged

Fix read raw limit #4370

merged 1 commit into from
Nov 6, 2023

Conversation

honnix
Copy link
Member

@honnix honnix commented Nov 6, 2023

Tracking issue

Closes #4368

Describe your changes

Compare limit at byte level to avoid accuracy loss when converting to MB.

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Screenshots

NA

Note to reviewers

NA

Copy link

welcome bot commented Nov 6, 2023

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

  • Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
  • Sign off your commits (Reference: DCO Guide).

@honnix honnix closed this Nov 6, 2023
@honnix honnix reopened this Nov 6, 2023
Signed-off-by: Hongxin Liang <honnix@users.noreply.github.com>
Copy link

codecov bot commented Nov 6, 2023

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (9fe34db) 59.07% compared to head (7648a2e) 59.35%.
Report is 1 commits behind head on master.

❗ Current head 7648a2e differs from pull request most recent head c74190a. Consider uploading reports for the commit c74190a to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4370      +/-   ##
==========================================
+ Coverage   59.07%   59.35%   +0.27%     
==========================================
  Files         614      544      -70     
  Lines       52070    38975   -13095     
==========================================
- Hits        30760    23133    -7627     
+ Misses      18859    13562    -5297     
+ Partials     2451     2280     -171     
Flag Coverage Δ
unittests ?

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

Files Coverage Δ
...ytepropeller/pkg/controller/nodes/array/handler.go 70.20% <50.00%> (+0.57%) ⬆️
...g/controller/nodes/array/node_execution_context.go 62.26% <60.00%> (+2.94%) ⬆️
flyteplugins/go/tasks/plugins/array/catalog.go 47.71% <75.00%> (+2.41%) ⬆️

... and 549 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@honnix
Copy link
Member Author

honnix commented Nov 6, 2023

@hamersaw Thanks for approving. Could you please help merge it? Thanks.

@kumare3
Copy link
Contributor

kumare3 commented Nov 6, 2023

Wow

@kumare3 kumare3 merged commit 939eb8b into flyteorg:master Nov 6, 2023
41 checks passed
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.

[BUG] GetLimitMegabytes check in stow_store.go is incorrect due to integer and float conversion
3 participants