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

do not log if omero #3577

Merged
merged 2 commits into from
Jun 30, 2020
Merged

do not log if omero #3577

merged 2 commits into from
Jun 30, 2020

Conversation

jburel
Copy link
Member

@jburel jburel commented Jun 23, 2020

Do not log id if omero
cc @dgault @sbesson

see: ome/omero-insight#136

@joshmoore
Copy link
Member

Added a link to the original issue so we have some context.

@joshmoore
Copy link
Member

@jburel : would another option be to only print the first line everywhere? If this had been formatted as an URI then parsing with that class would allow us to strip out parameters, but that generalization doesn't work. If ImageJ's file/url-protocol is newline separated, though, then that may be a more general solution.

@jburel
Copy link
Member Author

jburel commented Jun 24, 2020

In the case of OMERO, it does not really offer much, since it also printed out without the password immediately after, admittedly in the debug mode i.e. there was a lot of redundancy in the printing out.
We also already have the initial connecting step recored when the user logs in to the server when using the plugin.

For the formatting, I don't think it is wise to change it. It will require a review of the component using it, including scifio. This is for another review in my view

@joshmoore
Copy link
Member

Sorry, I don't follow. I agree that all the other locations need work, too. What I'm wondering about is whether or not we can't do something more general than adding an isOmero method to a component which should have no knowledge of OMERO.

@jburel
Copy link
Member Author

jburel commented Jun 24, 2020

The isOmero is not a new check. I just made a method since it is now used in 2 places. I went for the less disruptive change.

@dgault dgault added this to the 6.5.1 milestone Jun 24, 2020
@dgault
Copy link
Member

dgault commented Jun 30, 2020

Following the discussion at the formats meeting Im happy to have this included in 6.5.1 using the 2 private methods

@dgault dgault merged commit 65be096 into ome:develop Jun 30, 2020
@imagesc-bot
Copy link

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/imagej-omero-plugins-shows-password-in-log-window/39864/2

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.

5 participants