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

Added Invalid authentication for Git Remote to troubleshooting #2731

Merged
merged 18 commits into from
Sep 13, 2021

Conversation

daavoo
Copy link
Contributor

@daavoo daavoo commented Aug 18, 2021

Core P.R: iterative/dvc#6493

@shcheklein shcheklein temporarily deployed to dvc-org-dvc-exp-ssh-ihkxxryi9y August 18, 2021 09:21 Inactive
@daavoo daavoo self-assigned this Aug 18, 2021
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.

One thing to apply in the 3 docs ☝️

Also probably we should update the short description of git_remote under Synopsis (and then we can make an issue in the core repo for the corresponding update).

Thanks @daavoo

@shcheklein shcheklein temporarily deployed to dvc-org-dvc-exp-ssh-ihkxxryi9y August 26, 2021 08:00 Inactive
@daavoo daavoo mentioned this pull request Aug 26, 2021
2 tasks
@pmrowla
Copy link
Contributor

pmrowla commented Aug 26, 2021

One important note here: iterative/dvc#6493 (comment)

It doesn't have to be an SSH URL in every case though.

The SSH requirement is specifically for pushing (writing) to a remote git server that requires authentication (meaning pushing to github/gitlab). exp pull and exp list will work with any git URL (including HTTP) as long as the git server is public. If you are hosting a personal git server somewhere that was configured to allow pushing over unauthenticated HTTP, you could use the HTTP git URL format (and would not be required to use SSH).

And yes, read operations that require authentication (pull/list for private github/gitlab repos) would require using SSH.

@daavoo daavoo marked this pull request as draft August 27, 2021 10:30
@jorgeorpinel jorgeorpinel removed their request for review August 31, 2021 09:31
@jorgeorpinel
Copy link
Contributor

It doesn't have to be an SSH URL in every case though.

Sounds like this needs another rewrite then? To clarify

@daavoo
Copy link
Contributor Author

daavoo commented Aug 31, 2021

Yep. Also I think that some catch and show an error message pointing to docs might be added in dvc core.

@shcheklein shcheklein temporarily deployed to dvc-org-dvc-exp-ssh-ihkxxryi9y August 31, 2021 11:43 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-dvc-exp-ssh-ihkxxryi9y September 6, 2021 17:27 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-dvc-exp-ssh-ihkxxryi9y September 6, 2021 18:03 Inactive
@daavoo daavoo marked this pull request as ready for review September 6, 2021 18:04
@daavoo
Copy link
Contributor Author

daavoo commented Sep 6, 2021

One important note here: iterative/dvc#6493 (comment)

It doesn't have to be an SSH URL in every case though.
The SSH requirement is specifically for pushing (writing) to a remote git server that requires authentication (meaning pushing to github/gitlab). exp pull and exp list will work with any git URL (including HTTP) as long as the git server is public. If you are hosting a personal git server somewhere that was configured to allow pushing over unauthenticated HTTP, you could use the HTTP git URL format (and would not be required to use SSH).
And yes, read operations that require authentication (pull/list for private github/gitlab repos) would require using SSH.

Updated the warning to better clarify that

@shcheklein shcheklein temporarily deployed to dvc-org-dvc-exp-ssh-ihkxxryi9y September 6, 2021 18:05 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-dvc-exp-ssh-ihkxxryi9y September 7, 2021 07:43 Inactive
@jorgeorpinel jorgeorpinel added the ⌛ status: wait-core-merge Waiting for related product PR merge/release label Sep 7, 2021
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-dvc-exp-ssh-ihkxxryi9y September 7, 2021 23:55 Inactive
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
@shcheklein shcheklein temporarily deployed to dvc-org-dvc-exp-ssh-ihkxxryi9y September 8, 2021 09:51 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-dvc-exp-ssh-ihkxxryi9y September 9, 2021 09:01 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-dvc-exp-ssh-ihkxxryi9y September 9, 2021 09:19 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-dvc-exp-ssh-ihkxxryi9y September 9, 2021 09:21 Inactive
@jorgeorpinel jorgeorpinel removed the ⌛ status: wait-core-merge Waiting for related product PR merge/release label Sep 9, 2021
@@ -10,7 +10,7 @@ usage: dvc exp list [-h] [-q | -v] [--rev <rev>]
[git_remote]

positional arguments:
git_remote Optional Git remote name or repo URL
git_remote Optional Git remote name or authenticated Git URL
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just Git URL? Using authenticated here (and in the other places this change was added) doesn't make sense to me. This also makes it seem confusing since we don't mention authentication at all for Git remote name, but we do for Git URL (even though the potential auth requirements apply in both situations).

In the docs for remote add/remote modify we document URLs as

url - remote location

We don't say

url - authenticated remote location

@shcheklein shcheklein temporarily deployed to dvc-org-dvc-exp-ssh-ihkxxryi9y September 10, 2021 07:44 Inactive
@shcheklein shcheklein temporarily deployed to dvc-org-dvc-exp-ssh-ihkxxryi9y September 10, 2021 16:02 Inactive
Comment on lines +23 to +24
If a working `git_remote` name (e.g. `origin`) or Git URL is provided, lists
experiments in that <abbr>repository</abbr> instead (if any).
Copy link
Contributor

Choose a reason for hiding this comment

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

Git URL or file system path to the repo right? Should probably include all the options down here in the description similar to how the url arg is explained in https://dvc.org/doc/command-reference/import.

Copy link
Contributor

Choose a reason for hiding this comment

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

But I guess that's out of scope for the PR...

@jorgeorpinel jorgeorpinel added the ⌛ status: wait-core-merge Waiting for related product PR merge/release label Sep 11, 2021
@jorgeorpinel
Copy link
Contributor

Please merge along with iterative/dvc#6493.

@daavoo daavoo merged commit bbae045 into master Sep 13, 2021
@daavoo daavoo deleted the dvc-exp-ssh branch September 13, 2021 09:00
karajan1001 pushed a commit to karajan1001/dvc.org that referenced this pull request Sep 29, 2021
…rative#2731)

* Added note about [SSH Git URLs] in dvc exp commands

* Applied suggestions

* Updated warning

* Removed SSH from synopsis

* Added Invalid authentication for Git Remote to troubleshooting

* Added shorter title

* Applied P.R. suggestions

* Update content/docs/user-guide/troubleshooting.md

* Update content/docs/user-guide/troubleshooting.md

* Apply suggestions from code review

Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>

* valid -> authenticated

* Format

* Update troubleshooting.md

* Removed authenticated

* Added note about git credentials

Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
rogermparent pushed a commit that referenced this pull request Sep 30, 2021
* Added note about [SSH Git URLs] in dvc exp commands

* Applied suggestions

* Updated warning

* Removed SSH from synopsis

* Added Invalid authentication for Git Remote to troubleshooting

* Added shorter title

* Applied P.R. suggestions

* Update content/docs/user-guide/troubleshooting.md

* Update content/docs/user-guide/troubleshooting.md

* Apply suggestions from code review

Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>

* valid -> authenticated

* Format

* Update troubleshooting.md

* Removed authenticated

* Added note about git credentials

Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
rogermparent added a commit that referenced this pull request Oct 19, 2021
* Add tbd comment to pages/404.tsx

* Add comment to 404 page

* Update gatsby 2 to 3
* update needed dependencies to get gatsby 3 working
* add gatsby-plugin-image

* Update gatsby-source-github-api

* Take off tbd comment in 404.tsx

* Delete unneeded file in .github/workflows

* Fix attempted imported css warnings

* Add tbd comment to 404.tsx

* Rollback gatsby@3.10.2 to 3.6.0

* Remove comment in 404 page

* Move gatsby pkg back to 3.10.2

* Add tbd comment in 404 page

* Update outdated packages

* Delete tbd comment in 404 page

* Place console.log in DefaultSEO

* Run 'yarn upgrade'

* Delete comment in 404.tsx

* Upgrade @reach/portal

* Delete console.log in DefaultSeo

* Update @react/portal

* Upgrade some packages

* Update some packages

* Add tbd comment in 404.tsx

* Delete tbd comment

* Test static query

* Refactor useGlossary hook

* Move gatsby to 3.9.0

* Add tbd comment

* Update gatsby to 3.8.0

* Delete comment in 404 page

* Move gatsby to 3.6.0

* Add tbd comment

* Move gatsby to 3.4.0

* Delete comment in 404 page

* Move gatsby to 3.2.0

* Add tbd comment in 404 page

* Delete comment in 404 page

* Move gatsby to 3.0.0

* Add tbd comment in 404 page

* Update gatsby to 2.32.12

* Add tbd comment in 404 page

* Update gatsby to 3.14.0

* Move gatsby to 3.10.2

* Delete and recreate yarn.lock

* Delete tbd comment

* Update typescript to 4.4.2

* Update package.json and yarn.lock to match 1be35ce

* Add tbd comment

* Delete test query from siteMeta

* Delete console.log in siteMeta.ts

* Remove resolutions from package.json

* Delete tbd comment

* Upgrade gatsby

* Revert "Test static query"

This reverts commit 51a6012.

* Add PQR flag

* Upgrade gatsby-*-sharp and add lmdb-store

* Remove picture.small

* Update to node 16

* Add tbd comment

* Move gatsby to ^3.11.0

* Delete tbd comment

* Move gatsby to ^3.12.0

* Delete tbd comment

* Move gatsby to ^3.13.0

* Update some more packages

* Add dummy post

* Add ability to disable build retry in DEPLOY_OPTIONS

* Change content

* Change content

* Remove deprecated gatsby-image

* Try consecutive builds

* Undo consecutive builds experiment

* Log env

* Change content even more

* Remove dummy page

* Remove env display

* Add dummy page again

* Make an interim deploy script

* chmod +x

* Use vars and add test echos

* Remove echos and debug with which

* More debugging

* Even more debugging

* Remove debug stuff and mkdir that caused issues

* update content

* Remove dummy post

* Remove dummy blog post

* DVCLive User Guide updates (#2814)

* Updated top-level index. Refactored user-guide to top level

* Updated links

* Added dvclive-html.gif

* Explicit mention to automatic html update

* dvc 2.7.2 (#2813)

* dvc 2.7.2

* gha: update: delay by 2 hours

Co-authored-by: efiop <efiop@users.noreply.github.com>
Co-authored-by: Ruslan Kuprieiev <kupruser@gmail.com>

* Fix broken dvc downlaod link check (#2820)

* config: sync local/CI Prettier and Restyled (#2774)

* test: change content/docs/user-guide/contributing/blog.md
for #2641

* test: bad change to .../contributing/blog.md

* test: try reverting 2dd2e91#diff-eb41540235b5687201bb19ed968c284453eea68563f5908a6e15ed8cbfa959de

* format .restyled.yaml

* config: sort prettier settings by abc

* test: expanded Restyled config file
per #2774 (comment)

* Restyled by prettier (#2775)

Co-authored-by: Restyled.io <commits@restyled.io>

* guide: roll back dummy changes (fixes formatting)

* resyled: add comment on matching path pattern
per #2774 (review)

Co-authored-by: restyled-io[bot] <32688539+restyled-io[bot]@users.noreply.github.com>
Co-authored-by: Restyled.io <commits@restyled.io>

* Added `Invalid authentication for Git Remote` to troubleshooting (#2731)

* Added note about [SSH Git URLs] in dvc exp commands

* Applied suggestions

* Updated warning

* Removed SSH from synopsis

* Added Invalid authentication for Git Remote to troubleshooting

* Added shorter title

* Applied P.R. suggestions

* Update content/docs/user-guide/troubleshooting.md

* Update content/docs/user-guide/troubleshooting.md

* Apply suggestions from code review

Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>

* valid -> authenticated

* Format

* Update troubleshooting.md

* Removed authenticated

* Added note about git credentials

Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>

* dvc 2.7.3 (#2829)

Co-authored-by: efiop <efiop@users.noreply.github.com>

* gha: update: run every day at 13:00 UTC

* adding sept heartbeat (#2825)

* adding sept heartbeat

* punctuation, links, code revisions

* Update content/blog/2021-09-14-september-21-dvc-heartbeat.md

* adding cover image

* corrections to description

* fix the uploaded file link

Co-authored-by: David de la Iglesia Castro <daviddelaiglesiacastro@gmail.com>
Co-authored-by: rogermparent <rogermparent@gmail.com>
Co-authored-by: Ivan Shcheklein <shcheklein@gmail.com>

* Fix broken link to Azure authentication examples (#2830)

This link should point to: https://dvc.org/doc/command-reference/remote/modify#example-some-azure-authentication-methods

* gha: update download links at 18:00

* dvc 2.7.4 (#2837)

Co-authored-by: efiop <efiop@users.noreply.github.com>

* fix: correct argument order (#2842)

The order of arguments for `dvc remote rename` is reversed in the old version. 
According to the page @ `dvc remote rename -h`, the old remote `name` comes first followed by the `new` remote name argument.

* Rename all references to `get-started-experiments` to `example-dvc-experiments` (#2817)

Fixes #2784

* updating events page (#2847)

* Refactor post by Batuhan (#2835)

Co-authored-by: Batuhan Taskaya <isidentical@gmail.com>
Co-authored-by: Casper da Costa-Luis <casper.dcl@physics.org>

* Add --show-csv flag to dvc exp show (#2740)

* Add --show-csv flag to dvc exp show

* Apply suggestions from code review

* Format problem

Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>

* use folded block scalar type (#2871)

* Revert some unnecessary changes

* Some minor/patch package upgrades

* Add bug mitigation

* Disable PQR and bug mitigation

* Add USE_PRODUCTION_CACHE to deploy script for debugging

* Revert style import change and undo malformed comment

* Allow LayoutAlert to be false in typedef

* Remove nonstandard space and use width for spacing for lint-css

* Remove commented-out classnames

* Upgrade gatsby and some other minor/patch upgrades

* Add aptfile

* Revert "Remove commented-out classnames"

This reverts commit edae234.

* Gatsby 4 again

* Remove explicit typescript plugin and upgrade some packages

* Remove unused wrapper class

* Upgrade a bunch of packages

* upgrade plugin-feed, downgrade transformer-remark

* Update some more packages and try container stack

* Update some other packages to trigger a cache clear

* Revert stack change attempt, async setPageContext, remove Aptfile

* Upgrade CircleCI node and un-refactor glossary query

* Resolve ts-lint issues

* Prettier on content/docs/user-guide/contributing/blog.md

* Upgrade packages

* Remove composes from

* Remove source-github-api and other uses of deprecated runQuery

* Back to Gatsby 3

* Remove explicit webpack

* Use blur placeholder by default

* Fix misconfig on blur

* Upgrade some packages and remove unused lmdb-store

* yarn upgrade (lockfile only)

* Re-add typescript plugin definition (but not package.json)

* Re-add node_modules cacheDirectories

Co-authored-by: julieg18 <juliannagal18@gmail.com>
Co-authored-by: Julie <43496356+julieg18@users.noreply.github.com>
Co-authored-by: David de la Iglesia Castro <daviddelaiglesiacastro@gmail.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: efiop <efiop@users.noreply.github.com>
Co-authored-by: Ruslan Kuprieiev <kupruser@gmail.com>
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
Co-authored-by: restyled-io[bot] <32688539+restyled-io[bot]@users.noreply.github.com>
Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Jenifer De Figueiredo <jeny.defigueiredo@gmail.com>
Co-authored-by: Ivan Shcheklein <shcheklein@gmail.com>
Co-authored-by: Jose Celano <josecelano@gmail.com>
Co-authored-by: Gavin Masterson <51508128+GavinMasterson@users.noreply.github.com>
Co-authored-by: Emre Sahin <emre@iterative.ai>
Co-authored-by: Batuhan Taskaya <isidentical@gmail.com>
Co-authored-by: Casper da Costa-Luis <casper.dcl@physics.org>
Co-authored-by: Gao <mishanyo1001@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⌛ status: wait-core-merge Waiting for related product PR merge/release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants