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

Add parent ctr init call in InvalidArgumentError #9751

Merged
merged 1 commit into from
Jul 21, 2023
Merged

Conversation

shcheklein
Copy link
Member

@shcheklein shcheklein commented Jul 21, 2023

Fix for https://github.com/iterative/studio/issues/6897 (internal ticket).

Summary:

InvalidArgumentError: Can't use 'bad_name' as artifact name (ID). You can use letters and numbers, and use '-' as separator (but not at the start or end).
  File "dvc/repo/artifacts.py", line 65, in read
    check_name_format(name)
  File "dvc/repo/artifacts.py", line 32, in check_name_format
    raise InvalidArgumentError(

AttributeError: 'InvalidArgumentError' object has no attribute 'msg'

Bad name was causing a failure an unexpected AttributeError and breaking the parser in Studio.

Next steps

Separate followup question to discuss is why we allow these names silently. Tbh, I think it's better for us:

  • to fail fast here cc @daavoo @dberenbaum @aguschin what was the reason behind this decision?
  • also why don't we allow _ (it seems as natural as -)?

@codecov
Copy link

codecov bot commented Jul 21, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (41df96f) 90.45% compared to head (26c213f) 90.46%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9751      +/-   ##
==========================================
+ Coverage   90.45%   90.46%   +0.01%     
==========================================
  Files         480      480              
  Lines       36572    36585      +13     
  Branches     5259     5261       +2     
==========================================
+ Hits        33081    33098      +17     
+ Misses       2892     2889       -3     
+ Partials      599      598       -1     
Impacted Files Coverage Δ
dvc/exceptions.py 93.83% <100.00%> (+0.12%) ⬆️
tests/func/artifacts/test_artifacts.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@daavoo daavoo merged commit a9d0ac2 into main Jul 21, 2023
20 checks passed
@daavoo daavoo deleted the fix-exception-init branch July 21, 2023 06:27
@dberenbaum dberenbaum mentioned this pull request Jul 21, 2023
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