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

Remove redundant mongo #138

Merged
merged 14 commits into from
Oct 10, 2024
Merged

Remove redundant mongo #138

merged 14 commits into from
Oct 10, 2024

Conversation

yuema137
Copy link
Contributor

@yuema137 yuema137 commented Oct 7, 2024

Fix #136
The classes are redefined in straxen: straxen/storange/mongo_storage.py and all of the other xenon packages are only using the implementation in straxen (See #136 ). Given the independence of this module, it doesn't really matter if it's implemented in straxen or utilix. To avoid potential ambiguity in the future, a good practice is to remove the implementation here.

I have checked that the mongo_files module is independent of other modules in utilix, and it's safe to remove it.

Copy link
Contributor

@dachengx dachengx left a comment

Choose a reason for hiding this comment

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

@yuema137
Copy link
Contributor Author

@dachengx Please see the straxen PR: XENONnT/straxen#1442
and the private_nt_aux_files PR: https://github.com/XENONnT/private_nt_aux_files/pull/387

@yuema137 yuema137 requested a review from dachengx October 10, 2024 21:41
@dachengx
Copy link
Contributor

@dachengx Please see the straxen PR: XENONnT/straxen#1442 and the private_nt_aux_files PR: XENONnT/private_nt_aux_files#387

You need to solve the conflicts with the master branch.

@dachengx
Copy link
Contributor

Actually, haha, I suggest keeping these things to utilix but not straxen, because straxen is too heavy, and import straxen takes more resources.

Copy link
Contributor

@dachengx dachengx left a comment

Choose a reason for hiding this comment

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

Thanks @yuema137 Please later move RunDB related classes from straxen to utilix.

@dachengx dachengx merged commit eea8528 into master Oct 10, 2024
2 checks passed
@dachengx dachengx deleted the remove_redundant_mongo branch October 10, 2024 23:24
@yuema137
Copy link
Contributor Author

@dachengx Yes let's first gather the functions into one file in straxen, then move it to utilix. Then there's no need to modify private_nt_aux_files

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.

Is MongoUploader and MongoDownloader used?
2 participants