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

Convert OMERO strings to unicode #4

Merged
merged 5 commits into from
Sep 21, 2018

Conversation

manics
Copy link
Member

@manics manics commented Sep 17, 2018

Fixes https://trello.com/c/lLxHorjS/5609-prom-exporter-unicodedecodeerror

OMERO rstrings may contain unicode, but they are not automatically converted to Python unicode. This solution involved some trial-and-error.

Testing: omero-prometheus-tools.py -s pub-omero.openmicroscopy.org -u PRIVILEGED-USER -w PASSWORD -v -c etc/prometheus-omero-counts.yml against a server that has Unicode group names e.g. pub-omero and go to http://localhost:9449/

@manics
Copy link
Member Author

manics commented Sep 18, 2018

Added a test (uses modified omero-test-infra)

Though looks like travis isn't enabled on this repo.

Copy link
Member

@joshmoore joshmoore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, 👍 for dealing with unicode and 💯 for adding the tests.

Continuing the conversation from ome/omero-test-infra#22, I'm pretty sure this could be accomplished with test-data and lib-test files, but we can discuss that there.

Most relevant question here is if a unicode-aware unwrap method (or something else) would have helped you here.

Also, is "Login as микроскопом-пони fails with a unicode error so create" written up somewhere?

@manics
Copy link
Member Author

manics commented Sep 20, 2018

Thanks, I've got it working with test-data and lib-test based on ome/minimal-omero-client#27
There's a bug though: ome/omero-test-infra#24

@joshmoore
Copy link
Member

Closing and re-opening to trigger travis.

@joshmoore joshmoore closed this Sep 21, 2018
@joshmoore joshmoore reopened this Sep 21, 2018
@joshmoore
Copy link
Member

+docker run --link omeroprometheustools_omero_1:omero --net omeroprometheustools_default --rm -d -p 9449:9449 --name omero-prometheus-tools test -s omero -u root -w omero -c /opt/omero-prometheus-tools/etc/prometheus-omero-counts.yml -v
03f86f091f8ce3938bdc610b1ad31e629ce7a196c13f0f6ff9b0d6d24fec9a39
+sleep 10
+./test.py
PASSED

and weekly cron activated.

docker run --link $CID:omero --net $NETWORK --rm -d -p 9449:9449 --name omero-prometheus-tools test -s omero -u root -w omero -c /opt/omero-prometheus-tools/etc/prometheus-omero-counts.yml -v
sleep 10
./test.py
if [ "$NOCLEAN" != "true" ]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think this should probably be in the upstream lib-test and/or docker script as well.

@joshmoore
Copy link
Member

Nicely done. Shall his become 0.1.1?

@joshmoore joshmoore merged commit a653f63 into ome:master Sep 21, 2018
@manics
Copy link
Member Author

manics commented Sep 21, 2018

👍 for 0.1.1.

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