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

Allow for null baseline_sha #72

Closed
wants to merge 4 commits into from
Closed

Allow for null baseline_sha #72

wants to merge 4 commits into from

Conversation

dberenbaum
Copy link
Contributor

Needed for iterative/dvclive#646 to work without a baseline revision (since this would be a pain to provide in a no-git scenario).

@daavoo I will likely need your help to make similar changes on the Studio side to accept requests with missing baseline_sha.

@dberenbaum dberenbaum requested a review from daavoo August 4, 2023 20:48
@codecov-commenter
Copy link

Codecov Report

Patch and project coverage have no change.

Comparison is base (c85eda7) 100.00% compared to head (cd8507f) 100.00%.
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #72   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines          154       154           
  Branches         6         6           
=========================================
  Hits           154       154           

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

Copy link
Contributor

@daavoo daavoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use just a 40-character placeholder string like "z" * 40?

@dberenbaum
Copy link
Contributor Author

Can we use just a 40-character placeholder string like "z" * 40?

The schema checks for a valid sha here:

Sha = All(Lower, Match(r"^[0-9a-h]{40}$"), msg="expected a length 40 commit sha")

@shcheklein
Copy link
Member

@dberenbaum is it still relevant?

@dberenbaum
Copy link
Contributor Author

It depends on how we decide to go with issues like iterative/dvclive#638 and iterative/dvclive#237. If we decide to clone the repo on every node where dvclive runs, then this won't be needed, but if we decided to allow for some "no-git" scenarios, I think it might be needed. I can convert to draft until we decide.

@dberenbaum dberenbaum marked this pull request as draft September 8, 2023 18:42
@dberenbaum dberenbaum closed this Jan 26, 2024
@skshetry skshetry deleted the no-baseline branch March 6, 2024 07:17
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.

4 participants