Skip to content

Conversation

hsinfang
Copy link
Collaborator

I hesitate about this because adding a query with the central repo means time spent waiting for the database query, and it probably won't scale well. If we can agree just don't export raws in all cases, it seems better just to remove it. Do we really want to wait for DM-36051.?

@hsinfang hsinfang force-pushed the tickets/DM-39188 branch from 62b8ba2 to e1f089b Compare May 16, 2023 22:02
@hsinfang
Copy link
Collaborator Author

Change of mind after some Slack discussions. We can just not export raws at all.

@hsinfang hsinfang force-pushed the tickets/DM-39188 branch 3 times, most recently from 752bc91 to 73cf545 Compare May 18, 2023 16:56
Copy link
Member

@kfindeisen kfindeisen left a comment

Choose a reason for hiding this comment

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

The addition of second_local_repo looks fine in itself, but I don't see any tests that actually run interface and second_interface (or at least their export_outputs method?) in parallel. Then again, testing for parallelism issues is hard, since the test usually needs to put the system under a fair bit of stress...

@hsinfang
Copy link
Collaborator Author

Thanks for catching that. I'm adding it to test_export_outputs but indeed they aren't really run in parallel and I'm not sure how to enforce that in the current framework.

@kfindeisen
Copy link
Member

Would something go horribly wrong if you used multiprocessing from inside a test?

@hsinfang
Copy link
Collaborator Author

As a multiprocessing newbie.... if I just use it from inside a test to do a dummy test, it can pass. But using the middleware interface gave me TypeError: cannot pickle 'lsst.afw.cameraGeom._cameraGeom.Camera' object .
Will see whether I can avoid sending non-pickleble objects...

@kfindeisen
Copy link
Member

Sorry, I forgot that multiprocessing needs everything to be picklable. 😞 Since the actual framework never tries to share MiddlewareInterface objects, it's probably not worth designing them for it...

The two visits were tested using one shared local repos. Change it
so it is more similiar to the actual system.

This can already work before this ticket.
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.

3 participants