search

ncbi / docker

Other
48 stars 32 forks source link

Blast 2.14.0 updates for ease of maintenance and transparency #28

Closed boratyng closed 1 year ago

boratyng commented 1 year ago

I noticed this later and wanted to do the update before we forget till the next release. I do not think there is any reason to copy blastdb_path from the ftp site if we can build it from sources along with other binaries. Including binary wrappers in the repository will make docker images more transparent, safer and it will be one less manual step when creating images for new releases. It will also allow for changes in the wrappers. Currently a change may break a build of an older image.

christiam commented 1 year ago

Thanks @boratyng , this is a step in the right direction!

May I suggest to create a directory called common where we can store scripts that are re-used in subsequent releases to reduce duplication. We should also tag the repo so that things don't break in the future (e.g.: tags would be blast+2.14, blast+2.15, etc)

boratyng commented 1 year ago

@christiam , thanks for the suggestions, but I see a few problems.

May I suggest to create a directory called common where we can store scripts that are re-used in subsequent releases to reduce duplication.

I considered this, but are these files really common? Right now release versions are in directories, so to build an old version of the image a user would check out the master branch, go to the appropriate directory and build an image. Then these files will be common as long as they will not differ between versions. We can have them common for now, but that places a burden on us to remember to copy them when they change. Additionally the build-image.sh script would need to copy them before building an image. Currently, to make a new release, we copy the whole version directory, which would include the wrappers if they are there. I do not see keeping these files separate in a common directory as easier on the maintenance. On the contrary, it creates additional steps in the build process and we need to remember about not changing them.

We should also tag the repo so that things don't break in the future (e.g.: tags would be blast+2.14, blast+2.15, etc)

Yes, but what is then the point of versioned directories? We would be mixing two ways of releasing code: via directories and tags. I would prefer tags to directories, so maybe going forward we would change to tags, but then I would stop creating directories for new releases and relay on tags to get the code to build older image versions. Also then there is no need to keep several copies of the binary wrappers and no need for the. common directory.

christiam commented 1 year ago

On further thought it sounds like we should either keep things as they are or switch to tagged releases and having all of the files in one directory. I'm OK keeping things as they are.

boratyng commented 1 year ago

OK, we can at least build blastdb_path instead of downloading a binary and leave everything else as is. Here is a smaller pull request: https://github.com/ncbi/docker/pull/29.