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

bulk.yml depth is passed to xargs #337

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

will-moore
Copy link
Member

When performing bulk import, any depth: 100 value added to the bulk.yml is currently ignored and you have to use --depth=100 on the command line.

This attempts to fix that issue.

This fix works, BUT it now overrides any value passed on the command-line (which probably should take precedence)?
However, at this point in the code, I can't check xarg for the presence of any -Domero.import.depth command, since this is always there with the default of 4.

Not quite sure where the "right" place is to fix this?

cc @sbesson @joshmoore @dominikl

This will override any --depth=NNN passed on the command line
@sbesson
Copy link
Member

sbesson commented Oct 10, 2022

Looking briefly at the way the API are called, either the full xargs set-up or maybe only the omero.import.depth set-up could potentially be moved under do_import so that it's only defined once?

I would also have assumed that arguments passed via the command-line would take precedence as well but looking at the implementation, it is possible that this might not be the case as the parsing of the bulk keys calls CommandArguments.add for most keys and I do not see the logic for handling existing keys. Additional unit tests might be needed to confirm this.

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