MD-Anderson-Bioinformatics / NG-CHM

A dynamic, graphical environment for exploration of clustered or non-clustered heat map data in a web browser.
https://bioinformatics.mdanderson.org/main/NG-CHM-V2:Overview
GNU General Public License v2.0
10 stars 9 forks source link

Actions fix for releases #510

Closed marohrdanz closed 1 year ago

marohrdanz commented 1 year ago

This is a fix for the failed action during the release of 2.24.0.

Analysis showed the failure was due to the code trying to run on what appears to be a minified version of custom.js, and getting to the part for the cbioportal logo:

   logo: "https://www.cbioportal.org/images/cbioportal_logo.png",

This part includes the search string "images/", which is hard-coded into CompilerUtilities.java, therefore triggering an attempt to copy a file 'cbioportal_logo.png'. Of course that file does not exist in this repo, resulting in failure of the action.

For reasons I do not understand, changing the runner from macOS to ubuntu eliminates the problem (I tested in a personal fork of this repo).

In addition to this "fix", this pull request

ChrisWakefield commented 1 year ago

@jmelott To what are you referring? "runs-on: ubuntu-latest" ?

jmelott commented 1 year ago

@ChrisWakefield Yes, I was referring to "runs-on: ubuntu-latest".

ChrisWakefield commented 1 year ago

For reasons I do not understand, changing the runner from macOS to ubuntu eliminates the problem

@marohrdanz Sorry Mary. Too mysterious. I also looked at the code and the contents of the custom.js file. Is it possible custom.js became different for your personal fork version? (and the past versions?). I see no good reason why changing "runs-on" should give any different result.

Beyond this, CompilerUtilities.java leaves much to be desired.

marohrdanz commented 1 year ago

@jmelott I had those same concerns about 'latest'. My logic for using it was

I'm open to alternatives: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners

marohrdanz commented 1 year ago

@ChrisWakefield My instinct was some cached info, but experimentation suggested otherwise.

ChrisWakefield commented 1 year ago

@jmelott

Yes, I was referring to "runs-on: ubuntu-latest".

I think we want it to fail so the surprise comes as early as possible. Ideally, the condition you're flagging gets noticed before a production release. (...Hence Mary's change to "Adds workflow_dispatch to allow the workflow to be triggered manually").

That designation ("ubuntu-latest") is what github actions calls them. According to them: "Before moving the-latest label to a new OS version we will announce the change and give sufficient lead time for users to update their workflows". (https://github.com/actions/runner-images)

To clarify, I’m for leaving -latest in place (for THIS).

ChrisWakefield commented 1 year ago

We decided to proceed with the runner in ubuntu. Later we will restore using "-latest" for the runs-on, or not. An issue should be opened about macos and windows runners give a failure.