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

BF(workaround to #6759): if saving credential failed, just log error and continue #6762

Merged
merged 1 commit into from
Jun 14, 2022

Conversation

yarikoptic
Copy link
Member

This way user can still continue and achieve the goal without the entire
datalad process crashing and have no clear way forward. User would just
need to keep entering credential every time I guess.

Ref (not really a fix IMHO): #6759

This way user can still continue and achieve the goal without the entire
datalad process crashing and have no clear way forward.  User would just
need to keep entering credential every time I guess.

Ref (not really a fix IMHO): datalad#6759
@yarikoptic yarikoptic added UX user experience semver-patch Increment the patch version when merged labels Jun 10, 2022
@yarikoptic
Copy link
Member Author

appveyor - mac - test_ria_postclonecfg attn @bpoldrack . Otherwise - no side effects

@mih
Copy link
Member

mih commented Jun 13, 2022

Only tangentially related here, but the pattern established for the new-style credential handling is more different. It will only ever attempt to set a credential after it has proven to actually work. It would be great to figure out, how this could work with downloaders. So far, I failed to figure this out, which is blocking the introduction of REALM-based credential identification for this core functionality.

@yarikoptic
Copy link
Member Author

Indeed would be nice to make that happen. As for this particular change

  • would new credential mechanism crash process of unable to set credential?
  • does this change look ok?

@christian-monch
Copy link
Contributor

In my opinion, this is a good strategy.

Copy link
Contributor

@christian-monch christian-monch left a comment

Choose a reason for hiding this comment

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

Good strategy

@yarikoptic yarikoptic merged commit ab2bad0 into datalad:maint Jun 14, 2022
@github-actions
Copy link

🚀 PR was released in 0.16.6 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released semver-patch Increment the patch version when merged UX user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants