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

Protect against any stomp problems on import #60

Merged
merged 1 commit into from
May 21, 2021

Conversation

timj
Copy link
Contributor

@timj timj commented May 20, 2021

The stomp import can succeed but the .cvar access can fail.
Protect against all failures with stomp on import and only
set have_stomp on success.

With conda-forge stomp.py installed with 0.6.7.2 esutil we are seeing the following:

>>> import esutil
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Volumes/MediaHD/Users/timj/work/lsstsw/miniconda/envs/lsst-scipipe-0.6.0/lib/python3.8/site-packages/esutil/__init__.py", line 118, in <module>
    from . import stomp_util
  File "/Volumes/MediaHD/Users/timj/work/lsstsw/miniconda/envs/lsst-scipipe-0.6.0/lib/python3.8/site-packages/esutil/stomp_util.py", line 9, in <module>
    INSIDE_MAP = stomp.cvar.Map_INSIDE_MAP
AttributeError: module 'stomp' has no attribute 'cvar'

0.6.5 worked because it caught all exceptions. This PR reverts to that broader catch.

The stomp import can succeed but the .cvar access can fail.
Protect against all failures with stomp on import and only
set have_stomp on success.
@esheldon
Copy link
Owner

Is this some alternative version of stomp?

@timj
Copy link
Contributor Author

timj commented May 20, 2021

Yes: https://pypi.org/project/stomp.py/

which it turns out is used by our workflow submission system and so clashes with the stomp that esutil thinks it wants.

@esheldon
Copy link
Owner

I see. Thanks for the PR.

@esheldon esheldon mentioned this pull request May 21, 2021
@esheldon esheldon merged commit b87ee3f into esheldon:master May 21, 2021
@timj
Copy link
Contributor Author

timj commented Jul 21, 2021

@esheldon would it be possible to make a new release with this fix? I don't think I see a release since this PR was merged. We are currently having to pin to an old esutil version to allow our LSST pipelines to run at scale.

@timj timj deleted the u/timj/stomp-import branch July 21, 2021 18:36
@esheldon
Copy link
Owner

done, v0.6.8

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