Project

General

Profile

Actions

Bug #20862

closed

Deal with our google api client fork in the arvados ruby sdk

Added by Peter Amstutz over 1 year ago. Updated 7 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
API
Story points:
-
Release:
Release relationship:
Auto

Subtasks 2 (0 open2 closed)

Task #20864: Review https://github.com/arvados/google-api-ruby-client/pull/1ResolvedTom Clegg08/18/2023Actions
Task #20902: Review 20862-google-api-clientResolvedTom Clegg08/18/2023Actions

Related issues

Related to Arvados - Bug #21028: rocky8 package build failuresResolvedBrett SmithActions
Blocks Arvados - Idea #20300: RailsAPI upgrade from 5.2 to 7.0ResolvedTom Clegg10/06/2023Actions
Actions #1

Updated by Tom Clegg over 1 year ago

  • Status changed from New to In Progress
Actions #2

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
Actions #3

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?

Actions #4

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.

Actions #5

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.

Actions #6

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

Actions #7

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

Actions #8

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

Actions #9

Updated by Tom Clegg about 1 year ago

  • Blocks Idea #20300: RailsAPI upgrade from 5.2 to 7.0 added
Actions #10

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

Actions #11

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

Actions #12

Updated by Peter Amstutz about 1 year ago

  • Target version changed from Development 2023-08-30 to Development 2023-09-13 sprint
Actions #13

Updated by Peter Amstutz about 1 year ago

  • Target version changed from Development 2023-09-13 sprint to Development 2023-09-27 sprint
Actions #14

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.

Actions #15

Updated by Tom Clegg about 1 year ago

  • % Done changed from 50 to 100
  • Status changed from In Progress to Resolved
Actions #16

Updated by Brett Smith about 1 year ago

  • Related to Bug #21028: rocky8 package build failures added
Actions #17

Updated by Peter Amstutz 12 months ago

  • Release set to 67
Actions #18

Updated by Peter Amstutz 12 months ago

  • Release deleted (67)
Actions #19

Updated by Peter Amstutz 7 months ago

  • Release set to 70
Actions

Also available in: Atom PDF