Arvados

Plan:

• Update API server and Workbench packages to include a file with the version.
• The production section of application.default.yml should try to set source_version to the contents of that file.
• The production section of application.default.yml should set local_modified to false.

In production we currently have:

hieradata/uuid_cluster/common.yaml that has the version array including arvados-src

Brett what's your opinion about adding it to the application.yml.erb (in API and Workbench)?

this bug causes random fatal: Not a git repository (or any of the parent directories): .git when executing the console, see:

https://arvados.org/issues/7047#note-3

Comments for commit 44042f1b80c65e09aa94c738c0ac385b365b0669

• I think you can do a break in the "git_pipe.each_line do" loop after setting "status_output = true"

• It appears that the code block that computes source_version and local_modified at the top of api and workbench default yml file is the same. Would it be possible to create a new yml file such as "get_source_version.yml" with just this code block in one dir (let's say api/config) and link to it in workbench/config dir and include that file contents or results into the application.default.yml file?

Radhika Chippada wrote:

Comments for commit 44042f1b80c65e09aa94c738c0ac385b365b0669

• I think you can do a break in the "git_pipe.each_line do" loop after setting "status_output = true"

It's important to continue reading all of the output from the pipe. Otherwise, the git process may get killed by SIGPIPE and report errors when it would otherwise run fine.

• It appears that the code block that computes source_version and local_modified at the top of api and workbench default yml file is the same. Would it be possible to create a new yml file such as "get_source_version.yml" with just this code block in one dir (let's say api/config) and link to it in workbench/config dir and include that file contents or results into the application.default.yml file?

You're right that the code is identical between files. I would like to refactor it, but unfortunately changes like you suggest don't play well with our packaging infrastructure. Symlinks in the Workbench source are included verbatim in the Workbench packages, so symlinks to code outside the Workbench tree (i.e., in the API server) will become dangling links when the package is installed. That would make it impossible for the Workbench application.default.yml to include the code when installed from a package.

Brett: sounds good. LGTM (though I did not test by restarting my servers). Thanks.

Brett Smith wrote:

• The production section of application.default.yml should try to set source_version to the contents of that file.

I couldn't quite do this, because all the ERB in application.default.yml is run without regard to the current Rails environment. Trying to always read this file in the production section would break development instances.

But I can fake it by introspecting Rails.env directly. I've pushed a change that does this, and adds some comments to help clarify some of the issues Radhika brought up. Now at ffd589a.

I reviewed 744c0c3..f311437

LGTM

I verbally commented to Brett about the crons that excecute in random directories and we tried to come up with potential problems... but we think is safe enough...

Nico Cesar wrote:

I verbally commented to Brett about the crons that excecute in random directories and we tried to come up with potential problems... but we think is safe enough...

Specifically, I think our use of Bundler prevents the script from being launched outside the Rails root. But I decided better safe than sorry, and adjusted the code to make sure we always launch git from the Rails root. Pushed to master with that change in 285c0092. Thanks.

Reopening to rearrange this code so it doesn't cause trouble when non-Erb YAML parsers try to read application*.yml

This should really be a gem, meanwhile at least I've moved it to lib/app_version.rb

6967-yaml-format @ 965c5b3

(incl. bonus fix: "rake config:check" was printing the blob_signing_key value in the clear instead of *******)

Reviewing 6967-yaml-format:

• You should use git status --porcelain instead of git status -s, from the man page:

--porcelain Give the output in an easy-to-parse format for scripts. This is similar to the short output, but will remain stable across Git versions and regardless of user configuration.

• Suggest adding --untracked-files=no to git status so that the presence of untracked files don't cause it to report "modified"
• Turns out I had git-commit.version left over from package building, I was initially very puzzled why it was reporting the wrong version. Perhaps it should try git before looking for git-commit.version
• apps/workbench/lib/app_version.rb has the following copy and paste error at the top:
  

  # If you change this file, you'll probably also want to make the same
  # changes in apps/workbench/lib/app_version.rb.
  

  
  (not having two copies of the essentially the same file would be even better, but whatever.)

Peter Amstutz wrote:

• You should use git status --porcelain instead of git status -s, from the man page:

Sure, "regardless of user config" sounds better. Fixed.

• Suggest adding --untracked-files=no to git status so that the presence of untracked files don't cause it to report "modified"

Idk, seems to me if you have untracked files (that aren't .gitignored) then your source tree is modified. I'm thinking "added a new source file but haven't committed it yet". I'm guessing you are thinking of a different kind of example.

Either way, I'd like to stick to the bugfix here rather than turn it into an opportunity to request/add features. "Can't run python tests any more" is the reason this got reopened.

• Turns out I had git-commit.version left over from package building, I was initially very puzzled why it was reporting the wrong version. Perhaps it should try git before looking for git-commit.version

Hm, the subject here is "don't call git, at least in production" so I don't think that would fly.

Meanwhile, at least tests now fail if you have git-commit.version in your source tree, which could make that less puzzling than it was before.

• apps/workbench/lib/app_version.rb has the following copy and paste error at the top:

Fixed

(not having two copies of the essentially the same file would be even better, but whatever.)

Agreed

54f9295