-
Notifications
You must be signed in to change notification settings - Fork 9
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
Cli integration #98
Cli integration #98
Conversation
I believe you intended to merge into |
Yes, changed. Tomorrow I will tick off the other two items on the list and then it’s ready for review. |
…_clean_exit: Instantiate irods_conn in constructor
The issue I had with the search was a server issue. |
I changed:
|
There is still a small problem with the Cli. Now I changed the two arguments around the or, with the effect, that we cannot overwrite config parameters from command line. |
iBridgesCli.py
Outdated
@@ -71,6 +71,9 @@ def __init__(self, # pylint: disable=too-many-arguments | |||
self.config_file = None | |||
self.download_finished = None | |||
self.upload_finished = None | |||
self.irods_conn = None | |||
|
|||
default_irods_env = os.path.join(str(os.getenv('HOME')), '.irods', 'irods_environment.json') |
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.
Not sure os.getenv('HOME')
makes sense in a Windows environment. Maybe John's suggestion also takes care of that?
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.
The expanduser()
function/method in os.path
/pathlib.Path
convert ~
to the equivalent of the POSIX $HOME
variable. This works properly under Windows as well. The nice thing with pathlib
, the engine under LocalPath
, is that all the main POSIX concepts translate directly. This even includes the path separator /
. That's why using ~/.irods
and ~/.ibridges
works on Windows as well.
Just quickly checked compatibility between this PR and PR #96. Changes are only minor. It hardly makes any difference what is merged first. |
[96] Fix print to logging bugs Checked also compatibility with #98, only minor conflicts which can be solved.
Some bugs that occurred after last merge:
TODO:
This PR also fixes #93 and #94