nokogiri attributes workaround support for 1.6.x #32
Conversation
| @@ -42,7 +42,13 @@ def initialize conn | |||
| end | |||
|
|
|||
| def deserialize node, type=nil | |||
| type_attr = node['type'] | |||
| if type_attr = node['type'] | |||
There was a problem hiding this comment.
The empty if block here is weird. I would prefer:
type_attr = node['type'] || node.attributes['type'].valueThere was a problem hiding this comment.
I agree, there was some debug in there that just got removed just before I pushed.
Your suggestion will not always work as node.attributes['type'] doesn't always exist.
I'll change this awkward if statement. Thanks
|
Fixed my issue perfectly, please accept this pull request |
|
|
||
| # Work around for 1.5.x which doesn't populate node['type'] | ||
| # XXX what changed | ||
| if node.attributes['type'] and not type_attr |
There was a problem hiding this comment.
Would it make more sense to invert the two checks? I.e. first check that type_attr is nil and then check for node.attributes['type']?
There was a problem hiding this comment.
Hmm maybe.
There's either node['type'] or node.attributes['type']. This PR doesn't attempt to solve why there are discrepancies in the two versions of nokogiri. It's just a quick hack to allow it to work the latest version of nokogiri.
The current version Rbvmomi can support actually has a CVE against it, so the quicker this can be merged the better.
|
Any update on getting this merged in? |
|
Can we please get this merged in? This is an extremely low risk fix and I think a lot of people are waiting on this. I have tested this with my setup of vagrant-vsphere + rbvmomi 1.8.1 and nokogiri 1.5.10 |
nokogiri attributes workaround support for 1.6.x
|
Any chance to have a new release, or at lease publish this fix in a pre-release build? |
|
+1 we could definitely use an official release with the nokogiri fix. |
|
+1 for release |
|
A release would be awesome. Yes please. |
|
+1 for release, this is a pain to deal with |
|
+1 for release |
|
yet another +1 for release! |
|
+1 for release! We're blocked on this too.. |
|
+1 for a release |
|
Guys, how do you work around this bug without having a new public rbvmomi release? I'm using Vagrant and Vagrant-vsphere plugin. The plugin depends on rbvmomi, and that became a real issue. Vagrant is not compatible with nokogiri 1.5 anymore due to other dependencies, so I cannot continue to use the previous rbvmomi version. At the same time, I know no ways how to make the plugin requiring an alternate version of a gem. So I had to publish my own fork of rbvmomi at RubyGems, and maintain an additional fork of the plugin. |
|
+1 for a release |
|
+1 for release! We are using fog which relies on rbvmomi and are running into the same issue. |
|
This patch fixes the problem for me. Can we please get someone from VMware to release this? |
|
...or create some canoical fork otherwise? This is stalling since half a year 👎 |
|
Reached out to vmware people on Twitter, Hoping we'll get a release soon. |
|
@stonith I did the same, and hit a fling dev directly on irc as well. The feedback I got was pretty positive. I hope to see some results soon too. |
|
I blogged an open letter to vmware and included the lack of releasing this PR as evidence of vmwares's apathy of supporting this library. See the post for response from the gem owner. This resulted in some "offline" conversations that appeared quite optimistic at first but I am much less optimistic now that weeks have gone by with no action. I am going to ping some folks to see if anything can be done here but if not, I see no alternative than to fork and create a new authoritative repo and gem otherwise this project is essentially dead. |
|
Matt, we have had a very good internal discussion on this, I am quite positive about the outcome. Give us a bit more time as we work through logistics. |
|
Hey thats very encouraging. Thanks @cdickmann. Please keep this thread updated with progress. |
|
@mwrock 👍 |
|
👍 there are also some other pull requests open, that should be checked after the release. |
|
Since this is a new project for me and the turn around is really short, I've cut v1.8.2rc1 as a prerelease and I'll hold onto the gem file and only release to rubygems after I've heard back from a few of you that the gem is acceptable. I've set up v1.8.3 for a few weeks out. This will give us a chance to review the PRs and decide how to handle them and then release. After that, we'll move to long-term planning. |
|
Excellent news - thanks Shawn! |
|
I will test today. Thanks so much Shawn! |
|
I briefly tested this version with And, Matt, thank you for starting the process! |
|
Hi folks, based on helpful input on Going forward I'll build a release issue to capture any conversations about the release itself and not related to any other bugs. |
It appears the
node['type']field isn't being populated with later versions of nokogiri (ie 1.6.1). So the condition for finding the type based on this value fails. It can still be accessed vianode.attributeswhich seems to exist in both1.4.1to1.6.1.This should fix that AnyType VMODL error.
#31