Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
cmd: add parameters for configuration of WebDAV remotes #1617
cmd: add parameters for configuration of WebDAV remotes #1617
Changes from 6 commits
a00dd30
9864d50
6d7eeeb
127d00e
feb10a0
49e894d
4fa3d8b
8f0836c
b449189
84b76dc
e70cd5b
b76cec5
b413b0e
2f72006
d870b34
31bccb5
098fe82
5eca211
5d3b9ec
99c7b8d
0dbbb27
08f888f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it need quotes like other string params? I.e.:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know, thought it to be more like a variant of
user
, which has no quotes? But adding quotes should probably be fine. Are there guidelines what parameters are considered as strings/numerics/... and what formatting to apply to each?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's more of a command line parser thing. All strings can be wrapped with
"
but if they have special chars or spaces then you definitely need to wrap them. And in fact'
may be better... Idk.I'll just commit this!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, have you tried this? I assume you tested all the WebDAV params so you can probably tell or double check whether quotes are needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or if you can rec a quick WebDAV server software to install and run locally for testing, please share 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I think you are right, it is better to wrap this in
'
. There seems to be no standard defining the content and structure of tokens for WebDAV (at least it should be some kind of string). What I can tell for the service I am using (it is basically an onwcloud): it supports OAuth 2.0, probably bearer tokens, which might already contain some basic punctuation characters.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the tip.
ResolvingLeaving unresolved for future ref.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check this link: https://doc.owncloud.com/server/admin_manual/installation/docker/#docker-compose-yaml-file
Copy-paste that into a
docker-compose.yml
file and thendocker-compose up
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, thanks @skshetry! Will try this next time its needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these alternatives to
token
and touser/password
auth?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, according to the WebDAV client documentation (and that is also what I can infer from the implementation), these are additional parameters. But they are optional parameters, probably we should give this information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to say they're optional. Most parameters except what's shown in the
remote add
reference are assumed to be optional. But ideally we should briefly explain what these are for, or link to some WebDAV documentation.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added some explanation, is it enough? I don't know what I could link to, these are not really WebDAV specific options - they are just passed to the underlying
requests
API.WebDAV seems to be the only remote having such options, even though
HTTP
is also based onrequests
it does not expose these option?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that was more than enough. Thanks @iksnagreb!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. TBH Idk but maybe @efiop, who I always pick on for these questions 😬, does.