https://dev.arvados.org/https://dev.arvados.org/favicon.ico?15576888422019-01-10T18:51:07ZArvadosArvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=704972019-01-10T18:51:07ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li></ul> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=704982019-01-10T18:51:15ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>New</i></li></ul> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=705002019-01-10T18:51:23ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Related to</strong> <i><a class="issue tracker-6 status-3 priority-4 priority-default closed" href="/issues/13648">Idea #13648</a>: [Epic] Use one cluster configuration file for all components</i> added</li></ul> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=711642019-02-06T19:24:54ZTom Morristfmorris@veritasgenetics.com
<ul><li><strong>Target version</strong> changed from <i>To Be Groomed</i> to <i>Arvados Future Sprints</i></li><li><strong>Story points</strong> set to <i>1.0</i></li></ul> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=756622019-06-19T15:31:31ZTom Morristfmorris@veritasgenetics.com
<ul><li><strong>Target version</strong> changed from <i>Arvados Future Sprints</i> to <i>2019-07-03 Sprint</i></li></ul> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=756682019-06-19T15:33:32ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Assigned To</strong> set to <i>Peter Amstutz</i></li></ul> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=760352019-07-03T13:33:25ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Target version</strong> changed from <i>2019-07-03 Sprint</i> to <i>2019-07-17 Sprint</i></li></ul> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=763652019-07-17T14:12:05ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Target version</strong> changed from <i>2019-07-17 Sprint</i> to <i>2019-07-31 Sprint</i></li></ul> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=764272019-07-18T20:44:27ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>14717-ws-new-config @ <a class="changeset" title="14717: Migrate websockets to new config Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz@v..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/7388e2f812805496e67930fdd316bb19657cb9fa">7388e2f812805496e67930fdd316bb19657cb9fa</a></p>
<p>based on 14713-cds-new-config branch so that one probably needs to merge first.</p> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=764472019-07-22T14:33:54ZTom Cleggtom@curii.com
<ul></ul><p>debug printf</p>
<pre>
+ fmt.Printf("Clllllllllllient %v %v", *oc.Client, cluster.Services.Controller.ExternalURL)
</pre>
<p>LoadDefaults() seems like a testing-only thing ("zzzzz"), and is only used by one test -- maybe we could relegate this to the test package and let the test(s) do something like config.NewLoader(bytes.NewBuffer(arvadostest.DefaultConfigFile), nil)?</p>
Not sure about renaming PingTimeout to KeepaliveTimeout given "Keepalive" already has other meanings in both HTTP and TCP. SSH uses "server alive messages" and "client alive messages" to disambiguate. Even "ping timeout" isn't a great description -- we use it for two different things, 1) a ping interval and 2) a timeout for all sends/writes -- so perhaps
<ul>
<li>call it ClientAliveInterval and (potentially / in future) use it for SSH sessions, too? or</li>
<li>call it SendTimeout</li>
</ul>
<p>nit: should WebsocketsPath just be WebsocketPath?</p> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=764532019-07-22T18:59:14ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Tom Clegg wrote:</p>
<blockquote>
<p>debug printf</p>
<p>[...]</p>
</blockquote>
<p>Fixed.</p>
<blockquote>
<p>LoadDefaults() seems like a testing-only thing ("zzzzz"), and is only used by one test -- maybe we could relegate this to the test package and let the test(s) do something like config.NewLoader(bytes.NewBuffer(arvadostest.DefaultConfigFile), nil)?</p>
</blockquote>
<p>Removed. Added in a new behavior to make loading the main config optional (for backwards compatibility when only the component config is available) which has the side effect of providing a way to ask it for just the defaults.</p>
<blockquote>
Not sure about renaming PingTimeout to KeepaliveTimeout given "Keepalive" already has other meanings in both HTTP and TCP. SSH uses "server alive messages" and "client alive messages" to disambiguate. Even "ping timeout" isn't a great description -- we use it for two different things, 1) a ping interval and 2) a timeout for all sends/writes -- so perhaps
<ul>
<li>call it ClientAliveInterval and (potentially / in future) use it for SSH sessions, too? or</li>
<li>call it SendTimeout</li>
</ul>
</blockquote>
<p>Fixed.</p>
<blockquote>
<p>nit: should WebsocketsPath just be WebsocketPath?</p>
</blockquote>
<p>Fixed.</p>
<p>As noted above, revised loading behavior a bit to ensure that either/or/both the main config or component are available.</p>
<p>14717-ws-new-config @ <a class="changeset" title="14717: Fix ws tests Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz@veritasgenetics.com>" href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/b2df3cba90166bc67dd93600ac1a44ac6f9e81bd">b2df3cba90166bc67dd93600ac1a44ac6f9e81bd</a></p>
<p><a class="external" href="https://ci.curoverse.com/job/developer-run-tests/1409">https://ci.curoverse.com/job/developer-run-tests/1409</a></p> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=764542019-07-22T19:36:54ZTom Cleggtom@curii.com
<ul></ul><p>Is this "/etc/arvados/config.yml is optional if it doesn't exist and a per-component config was loaded" stuff really necessary? Especially since 1.4 is already telling everyone to move to /etc/arvados/config.yml, I'm thinking we can just write "make sure it exists" in the upgrading docs instead. I get the idea of surviving an upgrade even if you haven't read the docs, but I'm not sure it's worth the added special cases -- e.g., on a new system with no configs at all, it looks like c-d-s will error out with "legacy config file doesn't exist" which seems to point in the wrong direction; all code used by legacy components needs to avoid relying on the configured cluster ID because it might be the fake value "zzzzz"; "config-dump" still won't be able to tell you what config c-d-s is using until you create a [empty] cluster config file; etc.</p>
<p>I think we're better off making config loading work the same way regardless of who's asking (although there's already the exception that if you're using custom config file locations you need to use <code>config-dump -legacy-*</code> in order to match your <code>some-component -config</code> results if you're using alternate config file locations).</p> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=764822019-07-23T21:02:34ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Tom Clegg wrote:</p>
<blockquote>
<p>Is this "/etc/arvados/config.yml is optional if it doesn't exist and a per-component config was loaded" stuff really necessary? Especially since 1.4 is already telling everyone to move to /etc/arvados/config.yml, I'm thinking we can just write "make sure it exists" in the upgrading docs instead. I get the idea of surviving an upgrade even if you haven't read the docs, but I'm not sure it's worth the added special cases -- e.g., on a new system with no configs at all, it looks like c-d-s will error out with "legacy config file doesn't exist" which seems to point in the wrong direction; all code used by legacy components needs to avoid relying on the configured cluster ID because it might be the fake value "zzzzz"; "config-dump" still won't be able to tell you what config c-d-s is using until you create a [empty] cluster config file; etc.</p>
</blockquote>
<p>Ok, LegacyComponentConfig is gone. I was mostly trying to get run-tests.sh to work on jenkins in a backwards-compatible way without requiring $CONFIGSRC/config.yml but since we're going all in on config.yml anyway, I worked with Fernando and we did this instead: <a class="external" href="https://dev.arvados.org/issues/15492">https://dev.arvados.org/issues/15492</a></p>
<p>14717-ws-new-config @ <a class="changeset" title="14717: Remove LegacyComponentConfig behavior run-tests.sh now requires a minimal config.yml with..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/26c517777afbdb9668ab24aa61bfc44ecd7f26b9">26c517777afbdb9668ab24aa61bfc44ecd7f26b9</a></p>
<p><a class="external" href="https://ci.curoverse.com/job/developer-run-tests/1416/">https://ci.curoverse.com/job/developer-run-tests/1416/</a></p> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=764862019-07-24T13:29:50ZTom Cleggtom@curii.com
<ul></ul><p>Peter Amstutz wrote:</p>
<blockquote>
<p>I was mostly trying to get run-tests.sh to work on jenkins in a backwards-compatible way without requiring $CONFIGSRC/config.yml but since we're going all in on config.yml anyway, I worked with Fernando and we did this instead: <a class="external" href="https://dev.arvados.org/issues/15492">https://dev.arvados.org/issues/15492</a></p>
</blockquote>
<p>Ah. Maybe this is a tangent now, but... I see run-tests.sh also has some code to write its own config.yml. We also still have $temp/arvados.yml, written by run_controller() in run_test_server.py. Maybe we could simplify all this by just setting ARVADOS_CONFIG to $temp/arvados.yml the same way we set ARVADOS_API_TOKEN etc., so (except where we override it) everything uses the same config during tests?</p> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=764992019-07-24T20:28:42ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Tom Clegg wrote:</p>
<blockquote>
<p>Peter Amstutz wrote:</p>
<blockquote>
<p>I was mostly trying to get run-tests.sh to work on jenkins in a backwards-compatible way without requiring $CONFIGSRC/config.yml but since we're going all in on config.yml anyway, I worked with Fernando and we did this instead: <a class="external" href="https://dev.arvados.org/issues/15492">https://dev.arvados.org/issues/15492</a></p>
</blockquote>
<p>Ah. Maybe this is a tangent now, but... I see run-tests.sh also has some code to write its own config.yml. We also still have $temp/arvados.yml, written by run_controller() in run_test_server.py. Maybe we could simplify all this by just setting ARVADOS_CONFIG to $temp/arvados.yml the same way we set ARVADOS_API_TOKEN etc., so (except where we override it) everything uses the same config during tests?</p>
</blockquote>
<p>Alright, this required a bit of refactoring, but now run_test_server.py has a "setup_config" option which assigns ports for all the components, writes a config file, and exports ARVADOS_CONFIG to be used for the remainder of the test run. Various code involving endpoints now looks at Services.*.InternalURLs/ExternalURL in arvados.yml. I removed the code for reading/writing *.port files and various other bits that are obsoleted by the new config.</p>
<p>It's possible there are a few loose ends, I can't test on jenkins at the moment.</p>
<p>14717-ws-new-config @ <a class="changeset" title="14717: Delete obsolete functions from run_test_server.py Arvados-DCO-1.1-Signed-off-by: Peter Am..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/84058ef2f2142a2ced249d2bc02e8556ae268c6e">84058ef2f2142a2ced249d2bc02e8556ae268c6e</a></p> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=765012019-07-25T13:40:13ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>rebased 14717-ws-new-config @ <a class="changeset" title="14717: Delete obsolete functions from run_test_server.py Arvados-DCO-1.1-Signed-off-by: Peter Am..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/59a2d537f3450407aa48e32645d92a5246c046fe">59a2d537f3450407aa48e32645d92a5246c046fe</a></p> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=765182019-07-26T17:06:20ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>14717-ws-new-config @ <a class="changeset" title="15467: Explicitly configure for IPv4 Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz@veri..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/f89b45c97b047298c86dd58d5da8c07aa3d7d27e">f89b45c97b047298c86dd58d5da8c07aa3d7d27e</a></p>
<p>Passed tests (finally):</p>
<p><a class="external" href="https://ci.curoverse.com/view/Developer/job/developer-run-tests/1430/">https://ci.curoverse.com/view/Developer/job/developer-run-tests/1430/</a></p>
<p>Just waiting for comments / sign off.</p> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=765272019-07-26T20:34:27ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Updated for new config scheme:</p>
<p><a class="external" href="https://dev.arvados.org/projects/arvados/wiki/Hacking_prerequisites#Create-a-Postgres-user">https://dev.arvados.org/projects/arvados/wiki/Hacking_prerequisites#Create-a-Postgres-user</a></p> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=765312019-07-29T13:34:03ZTom Cleggtom@curii.com
<ul></ul><p>Peter Amstutz wrote:</p>
<blockquote>
<p>based on 14713-cds-new-config branch so that one probably needs to merge first.</p>
</blockquote>
<p>Is this still true? (Meanwhile I should be reviewing the diff 40e3249b1...639789ffd, right?)</p> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=765322019-07-29T13:44:42ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Tom Clegg wrote:</p>
<blockquote>
<p>Peter Amstutz wrote:</p>
<blockquote>
<p>based on 14713-cds-new-config branch so that one probably needs to merge first.</p>
</blockquote>
<p>Is this still true? (Meanwhile I should be reviewing the diff 40e3249b1...639789ffd, right?)</p>
</blockquote>
<p>No, I don't think the tests were passing on jenkins on the 14713 branch, that's how all the run_test_server.py and run-tests.sh wrangling started. Unless you think I need to start doing branch surgery to reorganize git history, at this point it I think it makes more sense to just merge 14717 which also has everything from 14713 in it.</p> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=765342019-07-29T14:46:57ZTom Cleggtom@curii.com
<ul></ul><p>Is there a reason for having ws and c-d-s use gopkg.in/yaml.v2 instead of our usual ghodss/yaml, or is this just goimports guessing wrong?</p>
run_test_server.py
<ul>
<li>Is there a use case for using different cluster IDs (other than zzzzz) in <code>$CONFIGSRC/config.yml</code>? It seems like we could just say <code>...["Clusters"]["zzzzz"]["PostgreSQL"]...</code> instead of <code>list(...["Clusters"].values())[0]["PostgreSQL"]</code> + automatically munging a non-testy-looking database name.</li>
<li>Along similar lines, is <code>/etc/arvados/config.yml</code> ever a reasonable place to put a test suite config file? Or could we simplify this by making CONFIGSRC mandatory? (Looks like we should update the description in run-tests.sh either way)</li>
<li>Curious, is there a reason for building the services part of the config 2 different ways (a big literal with ExternalURL, then some assignments to InternalURLs)?</li>
<li>This looks like a debug printf to remove:<br /><pre>+ print(ex, file=sys.stderr)</pre></li>
</ul>
In services/ws/server_test.go
<ul>
<li>TestLoadLegacyConfig should use c.Error() instead of log.Fatal() ... or even <code>c.Assert(err, check.IsNil)</code></li>
</ul> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=765432019-07-29T15:11:54ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Tom Clegg wrote:</p>
<blockquote>
<p>Is there a reason for having ws and c-d-s use gopkg.in/yaml.v2 instead of our usual ghodss/yaml, or is this just goimports guessing wrong?</p>
</blockquote>
<p>Yea that sounds like goimports or a cut and paste mistake, I didn't choose which yaml library to use intentionally.</p>
<blockquote>
run_test_server.py
<ul>
<li>Is there a use case for using different cluster IDs (other than zzzzz) in <code>$CONFIGSRC/config.yml</code>? It seems like we could just say <code>...["Clusters"]["zzzzz"]["PostgreSQL"]...</code> instead of <code>list(...["Clusters"].values())[0]["PostgreSQL"]</code> + automatically munging a non-testy-looking database name.</li>
<li>Along similar lines, is <code>/etc/arvados/config.yml</code> ever a reasonable place to put a test suite config file? Or could we simplify this by making CONFIGSRC mandatory? (Looks like we should update the description in run-tests.sh either way)</li>
</ul>
</blockquote>
<p>This behavior is there because the easiest place to get the postgres config in the arvbox environment is the /etc/arvados/config.yml that is already there. But I can make arvbox write a separate testing-only config file and point CONFIGSRC at it.</p>
<p>Alternately, /etc/arvados/config.yml could have two cluster definitions, one regular one, and one 'zzzzz'. But right now, I suspect lots of things would get confused because various things are relying on the assumption that there's exactly one item in the Clusters map and that key is the cluster id to use. How did you envision it working to tell components which cluster config to use when there's more than one?</p>
<blockquote>
<ul>
<li>Curious, is there a reason for building the services part of the config 2 different ways (a big literal with ExternalURL, then some assignments to InternalURLs)?</li>
</ul>
</blockquote>
<p>I was under the impression that you couldn't have expressions on the left hand side of dict entries in Python, but actually that seems to be a limitation of Javascript and/or Ruby, not Python. (ETOOMANYLANGUAGES)</p>
<blockquote>
<ul>
<li>This looks like a debug printf to remove:<br />[...]</li>
</ul>
</blockquote>
<p>I left that in on purpose because it seemed to be useful to know, but I can take it out.</p>
<blockquote>
In services/ws/server_test.go
<ul>
<li>TestLoadLegacyConfig should use c.Error() instead of log.Fatal() ... or even <code>c.Assert(err, check.IsNil)</code></li>
</ul>
</blockquote>
<p>Will fix.</p> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=765442019-07-29T15:25:07ZTom Cleggtom@curii.com
<ul></ul><p>Peter Amstutz wrote:</p>
<blockquote>
<p>This behavior is there because the easiest place to get the postgres config in the arvbox environment is the /etc/arvados/config.yml that is already there. But I can make arvbox write a separate testing-only config file and point CONFIGSRC at it.</p>
</blockquote>
<p>Aha. Yes, I think if arvbox has an implicit "use a named cluster's config but change dbname/id to test values" thing then it's better to keep that in arvbox, and keep run-tests simple ("here are the actual values to use").</p>
<blockquote>
<p>Alternately, /etc/arvados/config.yml could have two cluster definitions, one regular one, and one 'zzzzz'. But right now, I suspect lots of things would get confused because various things are relying on the assumption that there's exactly one item in the Clusters map and that key is the cluster id to use. How did you envision it working to tell components which cluster config to use when there's more than one?</p>
</blockquote>
<p>Yes, the idea is something along the lines of "set ARVADOS_CLUSTER_ID to abcde" so GetCluster() can choose the right one instead of requiring that only one is mentioned in the config file ... but I don't think we should tackle that now.</p> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=765452019-07-29T18:26:50ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Tom Clegg wrote:</p>
<blockquote>
<p>Is there a reason for having ws and c-d-s use gopkg.in/yaml.v2 instead of our usual ghodss/yaml, or is this just goimports guessing wrong?</p>
</blockquote>
<p>Fixed.</p>
<blockquote>
run_test_server.py
<ul>
<li>Is there a use case for using different cluster IDs (other than zzzzz) in <code>$CONFIGSRC/config.yml</code>? It seems like we could just say <code>...["Clusters"]["zzzzz"]["PostgreSQL"]...</code> instead of <code>list(...["Clusters"].values())[0]["PostgreSQL"]</code> + automatically munging a non-testy-looking database name.</li>
<li>Along similar lines, is <code>/etc/arvados/config.yml</code> ever a reasonable place to put a test suite config file? Or could we simplify this by making CONFIGSRC mandatory? (Looks like we should update the description in run-tests.sh either way)</li>
</ul>
</blockquote>
<p>Done.</p>
<blockquote>
<ul>
<li>Curious, is there a reason for building the services part of the config 2 different ways (a big literal with ExternalURL, then some assignments to InternalURLs)?</li>
</ul>
</blockquote>
<p>Done.</p>
<blockquote>
<ul>
<li>This looks like a debug printf to remove:<br />[...]</li>
</ul>
</blockquote>
<p>Fixed.</p>
<blockquote>
In services/ws/server_test.go
<ul>
<li>TestLoadLegacyConfig should use c.Error() instead of log.Fatal() ... or even <code>c.Assert(err, check.IsNil)</code></li>
</ul>
</blockquote>
<p>Fixed.</p>
<p>14717-ws-new-config @ <a class="changeset" title="14717: Put setup_config back into install_env, with comments Arvados-DCO-1.1-Signed-off-by: Pete..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/3596bdedbf0a592b3dd4bdcf589c3de7b8913ee1">3596bdedbf0a592b3dd4bdcf589c3de7b8913ee1</a></p>
<p><a class="external" href="https://ci.curoverse.com/view/Developer/job/developer-run-tests/1436/">https://ci.curoverse.com/view/Developer/job/developer-run-tests/1436/</a></p> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=765502019-07-29T19:16:48ZTom Cleggtom@curii.com
<ul></ul><p>Is there an advantage to preserving an existing setup_config result in start_services() rather than creating a new one? The condition seems to create a variant of the previous bug: interactive mode will generate a config once after this is merged, but never update it when the code changes. (Ideally "rake npm:install" wouldn't be so demanding about keep-web config in the first place, but ... ugh)</p>
<p>"CONFIGSRC environment variable not set to a config file" is a bit of a misdirection.</p>
<p>Rest LGTM</p> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=765542019-07-29T19:34:17ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Tom Clegg wrote:</p>
<blockquote>
<p>Is there an advantage to preserving an existing setup_config result in start_services() rather than creating a new one? The condition seems to create a variant of the previous bug: interactive mode will generate a config once after this is merged, but never update it when the code changes. (Ideally "rake npm:install" wouldn't be so demanding about keep-web config in the first place, but ... ugh)</p>
</blockquote>
<p>It will generate a config with new port numbers but none of the services should be running so I don't think it will mess anything up. I removed the conditional.</p>
<blockquote>
<p>"CONFIGSRC environment variable not set to a config file" is a bit of a misdirection.</p>
</blockquote>
<p>Made the validations & error messages more specific.</p>
<blockquote>
<p>Rest LGTM</p>
</blockquote>
<p>Thanks.</p> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=765682019-07-31T14:49:18ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Resolved</i></li></ul> Arvados - Feature #14717: [websockets] Use cluster confighttps://dev.arvados.org/issues/14717?journal_id=811692020-01-22T14:47:14ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Release</strong> set to <i>22</i></li></ul>