search

DistributedScience / Distributed-CellProfiler

Run encapsulated docker containers with CellProfiler in the Amazon Web Services infrastructure.
https://distributedscience.github.io/Distributed-CellProfiler/
Other
37 stars 24 forks source link

Add better support for plugins #134

Closed bethac07 closed 1 year ago

bethac07 commented 1 year ago

Allow users to grab a specific plugins repo commit, as well as to install

bethac07 commented 1 year ago

Still todo - logic for checking commit hash and requirements file are non-empty/good. (Maybe need to git pull in commit side also?)

bethac07 commented 1 year ago

We had a discussion and decided that we actually don't want to avoid failures when we are trying to checkout or install plugins things - because it might lead a user to think they're running a newer version when in fact it's an older version. So we'll try to add a nice error message but otherwise let it crash.

ErinWeisbart commented 1 year ago

LGTM. I added a commit to make configs backward compatible. That okay with you? (and then I ran black to make sure I hadn't introduced a mistake and it decided to make everything look different, but hey no mistakes...)

bethac07 commented 1 year ago

I added a commit to make configs backward compatible. That okay with you?

I think the odds that someone would have an updated run.py but NOT an updated config.py would be very low, since both changes were introduced in the same commit (you'd have to be manually updating your run.py), but no harm in being defensive. But as an FYI that's why I didn't originally do so, unlike in other cases where we've introduced new environment variables into the containers, since containers and run.py are more likely to be out of sync, it's more important to be defensive there.

and then I ran black to make sure I hadn't introduced a mistake and it decided to make everything look different, but hey no mistakes...

Ok! Let's just then NOT squash commits when merging this PR, which I usually do, but I don't want to confuse the two reasons for changes.

ErinWeisbart commented 1 year ago

I think the odds that someone would have an updated run.py but NOT an updated config.py would be very low, since both changes were introduced in the same commit (you'd have to be manually updating your run.py), but no harm in being defensive.

Makes logical sense, I was just picturing someone copying a config from project to project instead of editing the fresh config. If you think that's not something we need to protect against, you're welcome to revert my commits.

ErinWeisbart commented 1 year ago

One further thought - do we want to edit tag in Makefile to match? tag = 2.0.0_4.2.3 Would close #125