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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sdk): update report id validation and encoding #6203

Merged
merged 7 commits into from
Sep 1, 2023

Conversation

jo-fang
Copy link
Contributor

@jo-fang jo-fang commented Aug 31, 2023

Fixes

Description

What does the PR do?

馃 Generated by Copilot at acafbd1

Add validation and normalization for report_id parameter of Report class. Fix decoding issues caused by missing '=' character in report url.

Testing

How was this PR tested?

Checklist

  • Include reference to internal ticket "Fixes WB-NNNNN" and/or GitHub issue "Fixes #NNNN" (if applicable)
  • Ensure PR title compliance with the conventional commits standards

馃 Generated by Copilot at acafbd1

report_id checked
not empty, has equal sign
autumn of errors

@codecov
Copy link

codecov bot commented Aug 31, 2023

Codecov Report

Merging #6203 (ae1cc0d) into main (4e2e22f) will decrease coverage by 0.10%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6203      +/-   ##
==========================================
- Coverage   77.83%   77.74%   -0.10%     
==========================================
  Files         379      379              
  Lines       43763    43772       +9     
==========================================
- Hits        34062    34029      -33     
- Misses       9648     9690      +42     
  Partials       53       53              
Flag Coverage 螖
unittest 81.41% <100.00%> (-0.10%) 猬囷笍

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

Files Changed Coverage 螖
wandb/apis/reports/report.py 89.09% <100.00%> (+1.91%) 猬嗭笍

... and 18 files with indirect coverage changes

@jo-fang jo-fang changed the title add = fix(sdk): add base64 = back into report id Aug 31, 2023
@github-actions github-actions bot added cc-fix and removed cc-fix labels Aug 31, 2023
@jo-fang jo-fang marked this pull request as ready for review August 31, 2023 21:53
@github-actions github-actions bot added cc-fix and removed cc-fix labels Aug 31, 2023
@kptkin kptkin added this to the sdk-2023-09.1 milestone Sep 1, 2023
@jo-fang jo-fang changed the title fix(sdk): add base64 = back into report id fix(sdk): add base64 = padding back into report id Sep 1, 2023
@github-actions github-actions bot added cc-fix and removed cc-fix labels Sep 1, 2023
@kptkin
Copy link
Contributor

kptkin commented Sep 1, 2023

/fix-title

@github-actions github-actions bot changed the title fix(sdk): add base64 = padding back into report id fix(sdk): update report id validation and encoding Sep 1, 2023
@jo-fang jo-fang merged commit e1834ee into main Sep 1, 2023
81 checks passed
@jo-fang jo-fang deleted the joyce/report-from-url branch September 1, 2023 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants