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

Changed decoding to UTF-8 #366

Merged
merged 1 commit into from
Jan 22, 2020
Merged

Changed decoding to UTF-8 #366

merged 1 commit into from
Jan 22, 2020

Conversation

Peder2911
Copy link
Contributor

I changed the decoding done in the docker unpacker to UTF-8 from ascii.
The unpacker wasn't working for me until i did this (localization
issue?) Is there any reason not to use UTF-8 decoding ihere?

@remram44
Copy link
Member

Hi,

As far as I know, Docker only uses ASCII for image and container IDs. If this is required, it is probably that it read the wrong thing instead of the ID. Do you have a traceback or an example where an error happens?

@remram44 remram44 added the T-bug Type: A fix for unwanted behavior, NOT a new feature or a simple enhancement label Jan 21, 2020
@Peder2911
Copy link
Contributor Author

Hey! Thank you for the quick reply.

Yep, when trying to do reprounzip docker run i get the following traceback:

Traceback (most recent call last):
  File "/home/peder/Projects/Blackboxer/venv/bin/reprounzip", line 11, in <module>
    load_entry_point('reprounzip==1.0.16', 'console_scripts', 'reprounzip')()
  File "/home/peder/Projects/Blackboxer/venv/lib/python3.6/site-packages/reprounzip/main.py", line 144, in main
    args.func(args)
  File "/home/peder/Projects/Blackboxer/venv/lib/python3.6/site-packages/reprounzip/unpackers/common/misc.py", line 68, in wrapper
    return func(args)
  File "/home/peder/Projects/Blackboxer/venv/lib/python3.6/site-packages/reprounzip_docker-1.0.16-py3.6.egg/reprounzip/unpackers/docker.py", line 539, in docker_run
    outjson = json.loads(out.decode('ascii'))
UnicodeDecodeError: 'ascii' codec can't decode byte 0xf0 in position 3163: ordinal not in range(128)

The code that i am trying to run is really simple: It's just an R script that outputs a plot of some example data:

library(ggplot2)

cars <- read.csv("data/cars.csv")

plt <- ggplot(cars,aes(hp,disp))+
   geom_point()

ggsave("out/plot.png", plt, height = 3, width = 3, device = "png")

As i said, it works fine when i change encoding to UTF-8. My system has Norwegian localization, but i can't really see how that would affect docker?

@Peder2911
Copy link
Contributor Author

Same result with a different workflow: Trows same exception, but works after changing the decoding.

@remram44
Copy link
Member

remram44 commented Jan 22, 2020

Ah, it's the JSON! That makes more sense. I forgot that was also in there.

The `docker inspect` output contains a lot of information, which is not
necessarily all ASCII.
remram44 added a commit that referenced this pull request Jan 22, 2020
@remram44 remram44 merged commit 227cdcc into VIDA-NYU:1.0.x Jan 22, 2020
@remram44
Copy link
Member

Thanks for the fix @Peder2911, this decode() was incorrect! The JSON output from Docker contains all kinds of things in addition to container/image names.

Hopefully this is always UTF-8 and not locale-dependent, but this is not so easy to test (and who uses something else than UTF-8 nowadays?)

@Peder2911
Copy link
Contributor Author

I am so happy to have been able to contribute to this excellent project. You are doing great work @remram44!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: A fix for unwanted behavior, NOT a new feature or a simple enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants