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

Delete testOpenFiles #44

Merged
merged 1 commit into from Sep 20, 2016
Merged

Delete testOpenFiles #44

merged 1 commit into from Sep 20, 2016

Conversation

parejkoj
Copy link
Contributor

lsof doesn't exist in the same place on all systems, so I've converted to psutil which we use elsewhere.

I had to up the limit to current+10 because it was failing on my system with 5, so apparently psutil and lsof behave slightly differently?

nameList = [name[1:] for desc, name in procs if desc[0] == 'f' and desc[1:].isdigit()]
return nameList


def printOpenFiles():
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this function at all? It's not called.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's called in commented out blocks. I'll delete it and those.

testOpenFiles was checking astrometry.net behavior under small file handle
limits, but it usually "fails" by either segfaulting or erroring out in a way
that lmakes it impossible to debug what happened. Since it's an astrometry.net
problem, we should leave testing it to them.
@parejkoj parejkoj changed the title Converted to use psutil Delete testOpenFiles Sep 20, 2016
@parejkoj parejkoj merged commit 373cf1b into master Sep 20, 2016
@PaulPrice
Copy link
Contributor

The commit message says "Since it's an astrometry.net problem, we should leave testing it to them." That's not true --- the problem was in our use of astrometry.net.

@parejkoj
Copy link
Contributor Author

@PaulPrice Oh, interesting. That was not clear from the comments or other discussion. Do you think a test like this should continue to exist, and if so how should we make it useful e.g. actually report something helpful on failure, instead of segfaulting?

@PaulPrice
Copy link
Contributor

I think this is an important test because it caught our wrong use of astrometry.net (we weren't closing files; I think there was also a problem in astrometry.net). I wouldn't have thought that a test failing with a segfault was not "useful". It may not give additional information, but surely the fact that it fails catastrophically is useful information. We may be moving to different catalog formats, but we still support astrometry.net and most of our existing catalogs are in that format, so this test keeps our astrometry.net reading working.

@ktlim ktlim deleted the tickets/DM-7661 branch August 25, 2018 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants