On 04/19/2012 02:02 PM, Martyn Taylor wrote:
Hi Mo,
As we discussed I've had a few issues with conductor and aeolus-image with your patches on fedora 17. I tried to get this working with manual setup but still having issues. Unfortunately I'm going to have to nack this patch set until some of the issues are solved. I've put a list below with some output.
Regards
Martyn
- We have 1 test failure in aeolus-image.
Failures:
- Aeolus::Image::Warehouse::WarehouseModel#== when other object has
the same instance variables list with the same values Failure/Error: it { subject.==(other_warehouse_model).should be_true } expected false to be true # ./spec/models/warehouse/warehouse_model_spec.rb:41:in `block (5 levels) in module:Warehouse'
Finished in 0.45152 seconds 109 examples, 1 failure, 4 pending
Failed examples:
rspec ./spec/models/warehouse/warehouse_model_spec.rb:41 # Aeolus::Image::Warehouse::WarehouseModel#== when other object has the same instance variables list with the same values rake aborted!
Which environment are you testing this on? When I tried this on a stock F16 install w/ the patch applied to the latest aeolus-image HEAD all the tests pass.
On F17 this test fails both before and after this patch is applied so the patch doesn't break anything currently working. I looked into the issue and discovered that pre ruby-1.9 'instance_variables' return an array of string instance variable names, where as in ruby-1.9 an array of symbols is returned [1]. This will be fixed in a subsequent patch with the following change:
--- a/lib/aeolus_image/model/warehouse/warehouse_model.rb +++ b/lib/aeolus_image/model/warehouse/warehouse_model.rb @@ -40,7 +40,7 @@ module Aeolus return false unless instance_variables.sort == other_obj.instance_variables.sort # Otherwise, ensure that they're all the same instance_variables.each do |iv| - next if iv == "@obj" + next if iv == "@obj" || iv == :@obj return false unless other_obj.instance_variable_get(iv) == instance_variable_get(iv) return false unless other_obj.body == body end
Regardless, at this point I'm more concerned that everything still works in the environment which we are currently running / supporting than on a newer stack. Once the patches are in, we can continue testing this out end-to-end in the new environment.
- Conductor migrations fail:
rake aborted! (/home/martyn/aeolus-tests/conductor/src/config/locales/it.yml): couldn't parse YAML at line 386 column 54
Tasks: TOP => db:migrate (See full trace by running task with --trace) [martyn@vm1 src]$ rake db:migrate --trace Using gem require instead of bundler ========= cucumber/test env deps loaded... ========== ** Invoke db:migrate (first_time) ** Invoke environment (first_time) ** Execute environment Failed to load /home/martyn/aeolus-tests/conductor/src/config/oauth.json, continuing without OAuth support. Run rake dc:oauth_keys and restart Conductor to enable. ** Execute db:migrate rake aborted! (/home/martyn/aeolus-tests/conductor/src/config/locales/it.yml): couldn't parse YAML at line 386 column 54
Same comment as above. In any case, this looks like it is a similar issue to the ones fixed with patch 2/4 in the conductor set ("properly quote strings") just in a new locale (one of the ones recently introduced). In any case the fixes from the patch should still be valid for the new locales (will incorporate these into the updated set).
- We require shadow rubygem for aoelus-configure, puppet dep
Yes, ruby-shadow which puppet currently depends on is incompatible w/ Ruby 1.9. rubygem-shadow is being submitted to Fedora (BZ#782560) to replace it at which point puppet will be updated to depend on that.
Since these patches are for the upstream code base, and we aren't building RPMS for F17+ yet, this shouldn't be a blocker to get these patches in. You should be able to pull rubygem-shadow directly from the BZ if you want to test configure atm.
- Need to update the conductor spec to remove the fastercsv dependency
Patch 1/4 of the conductor patches in this set ("update conductor to ruby 1.9") does this:
+%if 0%{?fedora} > 16 +Requires: rubygem(net-ldap) +%else +Requires: rubygem(fastercsv) +Requires: rubygem(ruby-net-ldap) +%endif
In any case, would you mind having another go at the set, primarily focusing on make sure nothing is broken on F16 / ruby 1.8 / our current environment. If all is green on that front, we can just push this and start testing F17 out for a new Aeolus community release.
Appreciate it, -Mo
[1] http://blog.grayproductions.net/articles/getting_code_ready_for_ruby_19