Bug #20862
closedDeal with our google api client fork in the arvados ruby sdk
Related issues
Updated by Tom Clegg over 1 year ago
20862-update-deps @ https://github.com/arvados/google-api-ruby-client/pull/1
Our test tooling doesn't currently support running arvados tests on Jenkins using an unpublished gem, but I have installed this manually in my test environment as 0.8.7.5 and run apps/workbench, services/api, and services/login-sync tests successfully.
source:services/login-sync doesn't commit Gemfile.lock (our auto-version-string magic causes a chicken-and-egg problem which was resolved in 5e55bbfce418bbdbd5069a4b6ba14539815e764c by gitignoring Gemfile.lock). I think this means that once we publish the arvados-google-api-client gem, subsequent builds of the login-sync package will either fail or (hopefully, if bundler is smart enough) stay at arvados-google-api-client 0.8.7.4, because login-sync specifies an old version of signet that's incompatible with arvados-google-api-client 0.8.7.5.
With that in mind, I think the next steps are- review & merge this branch
- publish 0.8.7.5 to rubygems
- test 20862-google-api-client on Jenkins
- review 20862-google-api-client
- publish arvados gem to rubygems (as 2.6.4rc1??)
- update Gemfile and Gemfile.lock files in 20862-google-api-client to reference the published gem
- merge 20862-google-api-client
- have a good cry about it all
Updated by Tom Clegg over 1 year ago
I also explored the prospect of merging the (Apache-2.0) arvados-google-api-client fork into the arvados repo as sdk/ruby-google-api-client, and adding instructions to build/run-tests.sh so Jenkins dev tests test everything else (sdk/ruby, services/api, etc.) against the current/unpublished version instead of the latest published version. I got that working locally, but couldn't push the branch because of course none of the files in the previous commits have copyright headers. I think we could get around that by squashing the subtree merge and either adding the copyright headers or adding sdk/ruby-google-api-client/* to .licenseignore.
Worth doing that?
Updated by Brett Smith over 1 year ago
Tom Clegg wrote in #note-3:
I also explored the prospect of merging the (Apache-2.0) arvados-google-api-client fork into the arvados repo as sdk/ruby-google-api-client, and adding instructions to build/run-tests.sh so Jenkins dev tests test everything else (sdk/ruby, services/api, etc.) against the current/unpublished version instead of the latest published version. I got that working locally, but couldn't push the branch because of course none of the files in the previous commits have copyright headers. I think we could get around that by squashing the subtree merge and either adding the copyright headers or adding sdk/ruby-google-api-client/* to .licenseignore.
Worth doing that?
The idea of being able to test this stack locally seems really appealing to me. Having this infrastructure seems like it would make future maintenance on the gem easier. So personally I'm inclined to say yes, given that you've already done 80% of the work.
One option might be to write a commit that adds the gem directory to .licenseignore
, then adds the gem commits on top of that, then adds a commit that adds license headers and removes the .licenseignore
entry. That would change all the commit hashes but it would otherwise retain the history, so it seems nicer than totally squashing all that.
Updated by Brett Smith over 1 year ago
Tom Clegg wrote in #note-2:
20862-update-deps @ https://github.com/arvados/google-api-ruby-client/pull/1
My only comment on this is I think it would be a nice affordance if the exception messages had more detail, e.g., raise "multipart upload not supported by arvados-google-api-client"
. If they ever come up for us in future development, it seems likely they'll be deep in the stack, so specifics could help speed up a diagnosis.
Updated by Tom Clegg about 1 year ago
Added sdk/ruby-google-api-client to arvados repo. I had to disable the copyright header checker to push the branch (I gave up on other workarounds when, even with "*" in .licenseignore
, our script crashed on an invalid utf-8 byte sequence).
Also updated the exception messages as suggested.
20862-google-api-client @ dfadcc1a6ccb7128784c4894cbd297272db094d9 -- developer-run-tests: #3789
Updated by Tom Clegg about 1 year ago
^ nearly everything used the new arvados-google-api-client gem... except our integration tests that use arvados-server boot
. I gave up and pushed 0.8.7.5 to rubygems using the API key from jenkins.
20862-google-api-client @ dfadcc1a6ccb7128784c4894cbd297272db094d9 -- developer-run-tests: #3794
Updated by Tom Clegg about 1 year ago
...and pushed arvados-2.7.0.dev20230824142549.gem to rubygems.
20862-google-api-client @ dfadcc1a6ccb7128784c4894cbd297272db094d9 -- developer-run-tests: #3807
20862-google-api-client @ 49638a3dc44b79c711d44443c1d03ee02360bbb5 -- developer-run-tests: #3809
20862-google-api-client @ 7415e01eb23989648fb4850b5a0569796973445f -- developer-run-tests: #3810
Updated by Tom Clegg about 1 year ago
- Blocks Idea #20300: RailsAPI upgrade from 5.2 to 7.0 added
Updated by Tom Clegg about 1 year ago
Fixed the arvados gem's i18n dependency (to not require version <1) and pushed it (and arvados-cli) to rubygems as 2.7.0.rc1 2.7.0.rc2.
With that, bundler is willing to update sdk/cli, apps/workbench, services/api, and services/login-sync to rails 6.1+. (To be clear, this branch doesn't do the rails update -- I just mean that I confirmed that with this branch, the stated dependencies in the arvados gem will no longer block the first "bundle update" step of #20300.)
20862-google-api-client @ 9a930f08a3b78221cea9cd90a8e8c9a77e3eb068 - developer-run-tests: #3811 -
20862-google-api-client @ 0043311bd95a8da91d0cc774542e7b008da46a42 -- developer-run-tests: #3812
Updated by Tom Clegg about 1 year ago
Reverted i18n upgrade -- rails 5 throws errors.
-------------------------------------------------------------------------------------------------------------- ApplicationControllerTest: test_#<JobsController:0x000055768d558ff8>_show_method_with_anonymous_config_enabled -------------------------------------------------------------------------------------------------------------- [...] #<ActionView::Template::Error: can not load translations from /home/tom/.gem/ruby/2.7.0/gems/activesupport-5.2.8.1/lib/active_support/locale/en.yml: #<ArgumentError: wrong number of arguments (given 2, expecte\ d 1)>> /home/tom/.gem/ruby/2.7.0/gems/i18n-1.14.1/lib/i18n/backend/base.rb:259:in `rescue in load_yml' /home/tom/.gem/ruby/2.7.0/gems/i18n-1.14.1/lib/i18n/backend/base.rb:251:in `load_yml' [...] /home/tom/.gem/ruby/2.7.0/gems/i18n-1.14.1/lib/i18n.rb:213:in `translate' /home/tom/.gem/ruby/2.7.0/gems/activemodel-5.2.8.1/lib/active_model/naming.rb:190:in `human' /home/tom/.gem/ruby/2.7.0/gems/actionview-5.2.8.1/lib/action_view/helpers/form_helper.rb:2264:in `submit_default_value' [...]
See also https://github.com/ruby-i18n/i18n/issues/410
20862-google-api-client @ 905c83db0b70400bd596388f5b3676adedcf8eb0 -- developer-run-tests: #3813
retry remainder (nginx port conflict) developer-run-tests-remainder: #4004
Updated by Peter Amstutz about 1 year ago
- Target version changed from Development 2023-08-30 to Development 2023-09-13 sprint
Updated by Peter Amstutz about 1 year ago
- Target version changed from Development 2023-09-13 sprint to Development 2023-09-27 sprint
Updated by Brett Smith about 1 year ago
Tom Clegg wrote in #note-11:
20862-google-api-client @ 905c83db0b70400bd596388f5b3676adedcf8eb0 -- developer-run-tests: #3813
I think this is basically good to merge, I have just a couple minor comments.
This problem predates the branch, but the fact that we have and rely on this fork seems to be basically undocumented. (If I missed something, please let me know.) It seems like we should have something somewhere that explains that our Ruby client components use this fork; the motivation for forking the original gem; and where the fork lives. I'm not totally sure where that should go. Maybe in the wiki; maybe in sdk/ruby-google-api-client/README
+ @Gemfile@s that use it. Doesn't have to be involved; each of those points could be covered by literally one sentence, I think.
Since we've forked the gem, we should ideally be testing it too. I don't expect that to happen in this branch, but it would be nice if we had a follow-up issue for that, and then test_sdk/ruby-google-api-client
could emit a warning that it was skipping tests with a pointer to that issue. I don't want someone to lose time thinking that run-tests.sh
is running the tests for them when it doesn't really.
Thanks.
Updated by Tom Clegg about 1 year ago
- % Done changed from 50 to 100
- Status changed from In Progress to Resolved
Applied in changeset arvados|4aa8d709fe8483c295a6d16dadffb5b11848e5b8.
Updated by Brett Smith about 1 year ago
- Related to Bug #21028: rocky8 package build failures added