Conversation
Pull Request Test Coverage Report for Build 23208476266Details
💛 - Coveralls |
|
Hi @bmaranville, It basically reverts orsopy/#65 (discussed in orsopy/#62) where slddb was put into orsopy for joint distribution. I don't see the need to make slddb server "distributable" as it is probably only installed once on our website. The slddb package in orsopy has for me two main advantages:
|
|
I agree with you about point 2. - this PR does not duplicate any code (and in fact it reduces duplication a little bit). The entire slddb code is moved from orsopy to this repo, almost completely unchanged. I also agree that we don't really need the server to be distributable - if you want to have a separate repo just for the server, that's fine with me too. I just thought the The motivations for this move:
I disagree that it's useful to put multiple semi-independent libraries into the All that being said, I started down this path as I was trying to fix an issue where the local query function is currently unable to handle e.g. DNA and RNA lookups and those are sent to the webquery instead. I have learned enough about where everything is (in the process of making this PR) that I think I could fix that issue in-place without moving the slddb code to a new repo. |
|
I'm fine with either solution (orsopy.slddb combined vs slddb as separate orso package). I don't think installing orsopy for slddb is an issue, as we have basically no extra requirements (orsopy requires pyyaml, nothing else), but I see you point about usage outside of orso. In case we do this separation, I'd prefer two separate repositories for the server and slddb as there are often purely web-related fixes I need to do on the server that don't change any business-logic. Refactoring all actual logic outside the server is also a good idea. (Now thinking about it a bit I'm no longer sure about separation, I'll look through the code a bit more to make up my mind. Maybe having a coupled server and slddb library version makes some sense.) Can you check with the ORSO crowd (e.g. by re-opening orsopy#62) to get consensus on this? I think it should also include some kind of backward compatible package resolution code to not break dependable programs. (E.g. GenX is using orsopy.slddb for material resolution.) |
This PR
slddb, which has the api for looking up all items from the databaseslddb_servera web-based application that uses that api (and others)orsopypackage to this one.A symmetric PR will be created in the orsopy repository, which removes the slddb code, and adds this slddb package as a requirement.