-
Notifications
You must be signed in to change notification settings - Fork 483
bakta, fix for local DB usage #7241
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
base: main
Are you sure you want to change the base?
Conversation
| <command detect_errors="aggressive"><![CDATA[ | ||
| mkdir -p ./database_path/amrfinderplus-db && | ||
| ln -s '$(input_option.bakta_db_select.fields.path)'/* database_path && | ||
| for file in '$(input_option.bakta_db_select.fields.path)'/*; do [ -f "\$file" ] && ln -s "\$file" database_path; done && |
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.
Why isn't this failing the test?
Is the path in the datatable a directory (please add this info to the loc file)? I guess so. If then
| for file in '$(input_option.bakta_db_select.fields.path)'/*; do [ -f "\$file" ] && ln -s "\$file" database_path; done && | |
| ln -s '$(input_option.bakta_db_select.fields.path)' database_path && |
should be the right thing to do, or?
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.
Hi @bernt-matthias ,
Not sure why it's not failing the test ? (I understand from your reply it should apparently :/ ). You mean the for loop itself or the use of the quotes + /* ?
Well the full issue is that when you use a local version of the bakta DBs, it already has that amrfinderplus-db subdirectory in it as well (otherwise the local cmdline version of bakta is not working as it needs that subdir in the path specified with the --database parameter) .
And the link cmd fails because the 'file' amrfinderplus-db already exists (==created in the step before) and blocks the making of a link
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.
Ah, you are hinting to just linking one dir to another, right?
I'll do some tests but I assume you'll end up in the same issue
(If you're using it via the data-managers from galaxy it needs to 2-step approach as then you 'll need to add-in the amrfinder subdir to the database_path (to get the required subdir in the main folder)
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.
'$(input_option.bakta_db_select.fields.path) is a directory indeed but you need to link on each individual files as simply linking the dirs does not work apparently.
Another dir needs to be included in the main dir database_path/ and therefore simply making a link to main dir does not work because then you'll try to make the amrfinderplus-db dir in the target dir (which not allowed obviously) .
ln -s '$(input_option.bakta_db_select.fields.path)' database_path &&
the above technically works but then you can't link the amrfinder-db directory in it (because database_path is already a link itself)
and if you execute the above when database_path is already created you'll link to the a new subdir (database_path/new_dir/ ) and thus not to simply database_path
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.
Hi @lsterck. I'm sorry, but I still do not understand the problem that you are trying to solve here.
When trying to use a local installed bakta DB the command block cmdline used in the wrapper failed, so this is an 'extension' of the command block to allow using local installed bakta databases.
- Why is it failing for you? What is the error?
- Why are the tests at IUC not failing?
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.
Hi @bernt-matthias ,
no worries, it is a bit of a complicated (or rather edge-case situation?) and my explanations are perhaps not super accurate either ;-)
Why is it failing for you? What is the error?
It is failing for me because I want to use the local version of the bakta-DB we have already.
Bakta needs 2 DBs, it's own bakta-db and the amrfinderplus-db, but they need to specified via a single database parameter, so the amrfinderplus-db is included, as a subdir, in the bakta-db location.
This particular setup is achieved with that 2-step procedure in the beginning of the command block in the wrapper (lines 12-14).
However, when I want to use our local version DB, the location of the bakta-db already includes the amrfinderplus-db subdir (otherwise our local, non-galaxy version of bakta is not working).
the error I get is: "ln: failed to create symbolic link './database_path/amrfinderplus-db': File exists"
Why are the tests at IUC not failing?
Because they don't use locally installed bakta-db that is configured for non-galaxy usage. It uses a bakta-db version that does NOT has the subdir amrfinderplus-db in it (it links in that amrfinderplus-db from a different location.
(I can copy that situation locally here as well, but than I have to make a local copy and remove the amrfinderplus-db from it, and link that in from a separate amrfinderplus-db version/location using the wrapper).
An additional level of added by linux/bash itself: you can't make a link where the link name is an existing file/dir . As such you need to use that syntax that is in the wrapper now ( '<some db location<'/* ): that will link each individual file rather than linking the directories to each other
FOR CONTRIBUTOR:
When trying to use a local installed bakta DB the command block cmdline used in the wrapper failed, so this is an 'extension' of the command block to allow using local installed bakta databases.
(briefly: changed linking by a for loop linking approach).
I'm available to provide more elaborate explanation if need-be