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

files_external:list --importable-format documentation #2675

Merged
merged 1 commit into from
Jun 12, 2020
Merged

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Jun 11, 2020

Fixes issue #2674
core code changes were merged in PR owncloud/core#37513 for 10.5.0

Backport to 10.5 only.

@phil-davis phil-davis requested a review from mmattel June 11, 2020 08:44
@phil-davis phil-davis self-assigned this Jun 11, 2020
@mmattel
Copy link
Contributor

mmattel commented Jun 11, 2020

From core:

A new option to files_external:list has been added - -i or --importable-format - when this
is specified and a JSON output is requested, the items above are corrected in the JSON
to be in a format that can be used for import.

This reads for me that -i needs a proper output format (json or json_pretty ?) to be defined - but not plain which is used by default.

@phil-davis
Copy link
Contributor Author

@mmattel any output format is valid. If used with a "normal" list then the text columns for Storage and Authentication Type show the "technical" values rather than the "human readable" values.

See owncloud/core#37513 (comment) - @jvillafanez also asked about this. It's good to see that reviewers are looking :)

@mmattel
Copy link
Contributor

mmattel commented Jun 11, 2020

I have tested it just now.
I propose to add a text below the table like:

...
TIP: When using option --importable-format, consider using --show-password additionally. Else the user and/or admin has to change all mount passwords post importing manually to make mounts fully functional.
...

This is because if you miss --show-password, an import needs post work for either the user and/or the admin !

@phil-davis
Copy link
Contributor Author

This is because if you miss --show-password, an import needs post work for either the user and/or the admin !

OK, I will look at it. The "problem" here is that the new --importable-format option has only been added so that files_external:export can switch it on. The availability to someone manually doing files_external:list --importable-format is really an accidental side-effect. There is no intention for files_external:list --importable-format by itself to produce the output ready for files_external:import - for that there is the command files_external:export which puts together all the pieces that are needed.

@mmattel
Copy link
Contributor

mmattel commented Jun 11, 2020

I think that the current implementation has a positive sideffect.
You have to mandatory add another option to make it fully work for importing.
This is a security measure ! 😄
The only thing is, it must be documented.
Just thinking about, instead of a tip, we can write NOTE: and start with: For security measures you have to add...

@phil-davis
Copy link
Contributor Author

@mmattel I wrote a "Note:" section

@phil-davis
Copy link
Contributor Author

Backport to 10.5 is PR #2678

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.

2 participants