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

Geometry from Karabo #330

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Geometry from Karabo #330

wants to merge 1 commit into from

Conversation

egorsobolev
Copy link
Member

This adds script requesting geometry from Karabo

@takluyver @JamesWrigley

install_requires=[
'Karabo-proxy',
Copy link
Member

Choose a reason for hiding this comment

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

Can we make this an optional dependency, using the extras_require section below? If people are installing our tools in their own environment, I try to keep the number of dependencies minimal.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, then I we also need to catch exception on import...

Copy link
Member

Choose a reason for hiding this comment

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

If you want to, but I think it's also OK to just let the ImportError show in that case.



def deserialize_geometry(serialized: str):
return pickle.loads(gzip.decompress(base64.b64decode(serialized.encode())))
Copy link
Member

Choose a reason for hiding this comment

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

I just realised what this is doing.

Unpickling may not work if the version of EXtra-geom installed in Karabo is different from the version locally. Recent versions haven't changed the structure much, but that's not guaranteed for the future, and the failures if you unpickle something from a different version can be very confusing. It also means that anyone calling this is potentially vulnerable to a MITM attack: unpickling is equivalent to eval() in security terms.

So I'd rather not do this if we can avoid it. If a major use case is to retrieve a .geom file, one option is that the geometry device could expose a slot to create the file, and we call that and write the returned data directly to a file.

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