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.

Sign up for GitHub

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

Jump to bottom

user-guide: add troubleshooting entry for dvc-exp in shallow clones #3577

Merged
merged 8 commits into from Jun 11, 2022
Merged

user-guide: add troubleshooting entry for dvc-exp in shallow clones #3577

merged 8 commits into from Jun 11, 2022

Conversation

pmrowla
Copy link
Contributor

@pmrowla pmrowla commented May 20, 2022 •
edited by jorgeorpinel

@pmrowla pmrowla self-assigned this May 20, 2022
@pmrowla
Copy link
Contributor Author

pmrowla commented May 20, 2022 •
edited by jorgeorpinel

This PR adds a troubleshooting entry with specific instructions for resolving the problem in CML, github actions, and gitlab CI/CD. It does not touch any of the command reference docs suggested in #3416 (comment), but IMO this should be sufficient to close the original issue, given that this is an edge case that is mostly specific to CI.

@gatsby-cloud
Copy link

gatsby-cloud bot commented May 20, 2022 •
edited

Gatsby Cloud Build Report

dvc.org

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 1m

Performance

Lighthouse report

Metric Score
Performance 🔶 59
Accessibility 💚 98
Best Practices 🔶 83
SEO 💚 93

🔗 View full report

@daavoo daavoo added ⌛ status: wait-core-merge Waiting for related product PR merge/release and removed ⌛ status: wait-core-merge Waiting for related product PR merge/release labels May 20, 2022
@dberenbaum
Copy link
Contributor

Is this intended to be in conjunction with iterative/dvc#7791?

@pmrowla
Copy link
Contributor Author

pmrowla commented May 21, 2022

Yes, this can probably be merged without waiting for the DVC PR though

@jorgeorpinel jorgeorpinel added the ⌛ status: wait-core-merge Waiting for related product PR merge/release label May 25, 2022
@jorgeorpinel
Copy link
Contributor

IMO this should be sufficient to close the original issue, given that this is an edge case that is mostly specific to CI.

Yes, if we're going with the error handling + troubleshooting section then there's probably no need to update other docs. Wouldn't hurt either but it's kind of a deep implementation detail.

jorgeorpinel
jorgeorpinel reviewed May 25, 2022
View reviewed changes
Copy link
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

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

Thanks for the content, all the info is great. I'm rephrasing and using tabs to clarify the alternative fixes (committing).

content/docs/user-guide/troubleshooting.md Outdated Show resolved Hide resolved
content/docs/user-guide/troubleshooting.md Outdated Show resolved Hide resolved
content/docs/user-guide/troubleshooting.md Outdated Show resolved Hide resolved
@restyled-io restyled-io bot mentioned this pull request May 25, 2022
Merged
@restyled-io @restyled-commits
Restyled by prettier (#3588)
6d692e8 
Co-authored-by: Restyled.io <commits@restyled.io>
@jorgeorpinel jorgeorpinel mentioned this pull request May 25, 2022
Closed
2 tasks
@jorgeorpinel jorgeorpinel self-assigned this May 25, 2022
@jorgeorpinel
Copy link
Contributor

Mind reviewing when you can @dberenbaum ? Need an approval (besides mine since I took it over). Thanks

@casperdcl casperdcl mentioned this pull request Jun 2, 2022
Merged
2 tasks
casperdcl
casperdcl previously requested changes Jun 2, 2022
View reviewed changes
content/docs/user-guide/troubleshooting.md Outdated Show resolved Hide resolved
content/docs/user-guide/troubleshooting.md Outdated Show resolved Hide resolved
content/docs/user-guide/troubleshooting.md Outdated Show resolved Hide resolved
content/docs/user-guide/troubleshooting.md

## DVC Experiments may fail in Git shallow clones {#git-shallow}

`dvc exp` commands use internal Git operations which may not work properly in
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe hyperlink to more info (presumably some section on reflog things?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Could use https://dvc.org/doc/user-guide/experiment-management/experiments-overview#how-does-dvc-track-experiments or https://dvc.org/blog/experiment-refs.

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't explain internal Git merges anywhere, not even in Peter's blog post. Too deep

content/docs/user-guide/troubleshooting.md Outdated Show resolved Hide resolved
content/docs/user-guide/troubleshooting.md Outdated Show resolved Hide resolved
@restyled-io restyled-io bot mentioned this pull request Jun 2, 2022
Merged
@julieg18 julieg18 temporarily deployed to dvc-org-troubleshooting-yvzuex June 2, 2022 12:18 Inactive
dberenbaum
dberenbaum approved these changes Jun 2, 2022
View reviewed changes
Copy link
Contributor

@dberenbaum dberenbaum left a comment

Choose a reason for hiding this comment

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

@jorgeorpinel I'm fine with it as is, but please take a look at the suggestions from @casperdcl before merging 🙏 .

@dberenbaum
Copy link
Contributor

It looks like iterative/dvc#6154 (comment) may be a similar issue, in which case maybe we should broaden this entry beyond dvc exp scenarios?

@dberenbaum dberenbaum removed the ⌛ status: wait-core-merge Waiting for related product PR merge/release label Jun 9, 2022
@dberenbaum
Copy link
Contributor

It looks like iterative/dvc#6154 (comment) may be a similar issue, in which case maybe we should broaden this entry beyond dvc exp scenarios?

Turns out this was unrelated.

@jorgeorpinel I think this is ready to be merged once the comments from @casperdcl are resolved.

@restyled-io @restyled-commits
Restyled by prettier (#3614)
d39764a 
Co-authored-by: Restyled.io <commits@restyled.io>
@jorgeorpinel @casperdcl
Apply suggestions from code review
035f093 
Co-authored-by: Casper da Costa-Luis <casper.dcl@physics.org>
@restyled-io restyled-io bot mentioned this pull request Jun 11, 2022
Merged
@shcheklein shcheklein requested a deployment to dvc-org-troubleshooting-yvzuex June 11, 2022 21:20 Abandoned
@restyled-io @restyled-commits
Restyled by prettier (#3638)
fe81a8f 
Co-authored-by: Restyled.io <commits@restyled.io>
@shcheklein shcheklein temporarily deployed to dvc-org-troubleshooting-yvzuex June 11, 2022 21:20 Inactive
@jorgeorpinel jorgeorpinel dismissed casperdcl’s stale review June 11, 2022 21:20

All of their commends are resolved.

@jorgeorpinel
Copy link
Contributor

comments*

@jorgeorpinel jorgeorpinel merged commit 1a50bd5 into master Jun 11, 2022
@jorgeorpinel jorgeorpinel deleted the troubleshooting-shallow branch June 11, 2022 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jorgeorpinel jorgeorpinel jorgeorpinel left review comments

@casperdcl casperdcl casperdcl left review comments

@dberenbaum dberenbaum dberenbaum approved these changes

Assignees

@pmrowla pmrowla

@jorgeorpinel jorgeorpinel

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

exp: CI runs require unshallow clone (fetch-depth: >1)
7 participants
@pmrowla @dberenbaum @jorgeorpinel @casperdcl @shcheklein @daavoo @julieg18