Bug #5989
closed
[API] API server repositories permissions list doesn't list repositories without keys
Added by Ward Vandewege almost 10 years ago.
Updated almost 10 years ago.
Description
bug 1: it does not create the 'auto' subdirectory if it does not exist yet:
Error: No such file or directory @ rb_sysopen - /usr/local/arvados/gitolite-tmp/gitolite-admin/conf/auto/XXXXX-s0uqq-asdfasdfadsfaafds.conf20150510-28428-1xbndn3.tmp
/usr/local/rvm/rubies/ruby-2.1.5/lib/ruby/2.1.0/tempfile.rb:146:in `initialize'
/usr/local/rvm/rubies/ruby-2.1.5/lib/ruby/2.1.0/tempfile.rb:146:in `open'
/usr/local/rvm/rubies/ruby-2.1.5/lib/ruby/2.1.0/tempfile.rb:146:in `block in initialize'
/usr/local/rvm/rubies/ruby-2.1.5/lib/ruby/2.1.0/tmpdir.rb:142:in `create'
/usr/local/rvm/rubies/ruby-2.1.5/lib/ruby/2.1.0/tempfile.rb:136:in `initialize'
/usr/local/arvados/update-gitolite.rb:62:in `new'
/usr/local/arvados/update-gitolite.rb:62:in `replace_file'
/usr/local/arvados/update-gitolite.rb:95:in `ensure_in_git'
/usr/local/arvados/update-gitolite.rb:103:in `ensure_in_git'
/usr/local/arvados/update-gitolite.rb:186:in `ensure_config'
/usr/local/arvados/update-gitolite.rb:242:in `block in <main>'
/usr/local/arvados/update-gitolite.rb:240:in `each'
/usr/local/arvados/update-gitolite.rb:240:in `<main>'
bug 2: it does not create any defined repositories on disk unless there is at least one ssh key associated with a user in the system. This is problematic because the 'arvados' repo is not created on disk.
- Description updated (diff)
- Subject changed from 2 small update-gitolite.rb script bugs to [API] Two small update-gitolite.rb bugs around new installs
- Category set to API
- Target version changed from Bug Triage to Arvados Future Sprints
- Target version changed from Arvados Future Sprints to 2015-07-08 sprint
- Assigned To set to Nico César
- Status changed from New to In Progress
brach 5989-bug_1 should thave the fix. How do we test this?
About bug 2 on line 233 I see:
permissions[:repositories].each do |repo_record|
repo = Repository.new(repo_record, user_ssh_keys)
repo.ensure_config(gitolite_admin)
end
That's Brett's code from 39b48294
why aren't we iterating over permissions instead of repositories?
a quick dirty fix will just be:
if not Repository.arvados_exists?
Repository.new("arvados")
repo.ensure_config(gitolite_admin)
end
but I want to make sure I'm doing the right thing and not an ugly patch and there are real reasons why we're doing that iteration.
Nico Cesar wrote:
About bug 2 on line 233 I see:
[...]
That's Brett's code from 39b48294
why aren't we iterating over permissions instead of repositories?
We are iterating over permissions instead of repositories, and that's exactly the problem: the API method that we call to get the source information does not return any information about repositories that don't have an associated SSH key. We should consider whether it makes more sense to change the API server to return that data—if we do, the script basically fixes itself. Alternatively, we could teach the script to query the full list of repositories, make sure they're all defined in gitolite, then propagate keys based on this API response—roughly along the lines of your second suggestion.
The code in 5989-bug_1 is good. See e-mail about deployment issues. Thanks.
- Subject changed from [API] Two small update-gitolite.rb bugs around new installs to [API] API server repositories permissions list doesn't list repositories without keys
Brett Smith wrote:
We are iterating over permissions instead of repositories, and that's exactly the problem: the API method that we call to get the source information does not return any information about repositories that don't have an associated SSH key. We should consider whether it makes more sense to change the API server to return that data—if we do, the script basically fixes itself.
Tom says yes. I will work on that change.
- Assigned To changed from Nico César to Brett Smith
Brett Smith wrote:
Brett Smith wrote:
We are iterating over permissions instead of repositories, and that's exactly the problem: the API method that we call to get the source information does not return any information about repositories that don't have an associated SSH key. We should consider whether it makes more sense to change the API server to return that data—if we do, the script basically fixes itself.
Tom says yes. I will work on that change.
Done in branch 5989-api-all-repos-permissions-wip, up for review.
reviewed 0e239ef805b8c6eda936073197cf96d0329db048
This the API change looks good to me.
BTW: I see
- User.includes(:authorized_keys).all.each do |u|
+ User.includes(:authorized_keys).find_each do |u|
nice. .find_each should be everywhere.
Nico Cesar wrote:
nice. .find_each should be everywhere.
Thanks for the review. find_each actually shouldn't be used everywhere, because it ignores limits and I think the ordering can get funky too. See 602706ef5510b3f07fc5fa988019952d2133320c for an example where using find_each was a bug that got fixed. But for cases like this where we know we're going to iterate everything, we don't need the whole list, and we don't care about order, yeah, it's much better.
- Status changed from In Progress to Resolved
- % Done changed from 50 to 100
Applied in changeset arvados|commit:5608a875c36101c791e35c474abf5e3900aad071.
Also available in: Atom
PDF
Updated by 
Updated by 
Updated by