pulp / pulp-openapi-generator

GNU General Public License v2.0
4 stars 24 forks source link

Pin faraday #59

Closed fao89 closed 2 years ago

fao89 commented 2 years ago

[noissue]

ekohl commented 2 years ago

Can you explain why this was needed? This is blocking us in the Foreman project and I don't see any reference of why this is needed.

fao89 commented 2 years ago

we didn't have a redmine for this repo, I believe the why was in some IRC conversation, I don't remember anymore

ekohl commented 2 years ago

Looking through my logs, I find this (removed irrelevant context):

--- Log opened Tue Jan 04 00:00:00 2022
21:35 < jsherrill> relevant messages we see: 
21:35 < jsherrill> WARNING: `Faraday::Connection#basic_auth` is deprecated; it will be removed in version 2.0.
21:35 < jsherrill> While initializing your connection, use `#request(:basic_auth, ...)` instead.
21:36 < jsherrill> and relevant pr: https://github.com/pulp/pulp-openapi-generator/pull/56/files
21:37 < jsherrill> dkliban[m] and i were digging into this last year, and i tested out the new bindings built with that, and they failed for other reasons
21:38 < jsherrill> we should probably pin faraday to < 2.0 untill we can upgrade to the new openapi generator
21:39 < dkliban[m]> oh yeah
21:39 < dkliban[m]> jsherrill: did a new version of faraday get released?
21:40 < bmbouter[m]> https://cdn.shopify.com/s/files/1/0851/8780/products/KOOLN_large.jpg?v=1578671064
21:40 < dkliban[m]> oh yeah ... Jan 4th
21:41 < ggainey> jsherrill++
22:37 < dralley> dkliban: could you review https://github.com/pulp/pulpcore/pull/1791 so that the CI is unblocked
22:39 < dkliban[m]> dralley: i approved it ... but we still need a fix for the bindings test
22:39 < dkliban[m]> we need to install an older version of faraday rubygem
22:39 < dkliban[m]> <2.0 
22:41 < dkliban[m]> i believe it is getting installed here https://github.com/pulp/plugin_template/blob/master/templates/github/.github/workflows/scripts/script.sh.j2#L80
22:42 < dkliban[m]> jsherrill: what's the best way to downgrade Faraday in our CI? 
22:42 < dkliban[m]> jsherrill: it gets installed when the ruby gem for the bindings is installed
--- Log closed Wed Jan 05 00:00:02 2022

--- Log opened Wed Jan 05 00:00:02 2022
14:16 < dkliban[m]> jsherrill: https://github.com/pulp/pulpcore/blob/main/.github/workflows/scripts/script.sh#L71
14:18 < jsherrill> dkliban[m]: i would try running this prior to that gem install line:   gem install faraday --version=1.8.0
14:18 < jsherrill> i *think* that it won't pull in a newer version 
14:18 < dkliban[m]> jsherrill: thank you
14:19 < dkliban[m]> ggainey: can you please try adding ^ to your PR ?
14:19 < dkliban[m]> if it works we can add it to the plugin_template
14:19 < ggainey> checking
14:20 < ggainey> ya can do, gimme a sec
14:24 < ggainey> um...it's already there?
14:24 < ggainey> oh
14:24 < ggainey> oh god, nm i r idiot
14:24 < ggainey> one sec
14:25 < ggainey> ok, pr updated
14:33 < x9c4[m]> lmjachky So removed the option completely? Or you remove "required" from the option?
14:34 < x9c4[m]> If the option disappeard, you can add a `needs_plugins=[PluginRequirement("bla", max="first-version-to-not-include-this"]` to the options kwargs.
14:35 < x9c4[m]> If it's still avail, the best we can do is check and fail in the `preprocess_body` based on the plugin version.
14:37 < ggainey> dkliban: no joy, alas : https://github.com/pulp/pulpcore/runs/4715120018?check_suite_focus=true#step:13:996
14:38 < ggainey> even tho https://github.com/pulp/pulpcore/runs/4715120018?check_suite_focus=true#step:13:623
14:39 < dkliban[m]> strange
14:40 < dkliban[m]> i guess we are not fully understanding what is going on
14:40 < ggainey> the logs show faraday==1.8.0 being installed, but the error shows 2.0.0 being found
14:40 < ggainey> wth
14:40 < dkliban[m]> i gotta take a quick break and then i have a meeting. i'll be able to troubleshoot this in about an hour
14:40 < ggainey> kk
14:40 < ggainey> i'll puzzle over it as well
14:56 < lmjachky[m]> <x9c4[m]> "lmjachky So removed the option..." <- completely
14:57 < x9c4[m]> Then a needs_plugins should do the trick. Make sure it's a `pulp_option`.
14:57 < lmjachky[m]> okay, thanks
15:01 < fao89[m]> dkliban: ggainey  https://github.com/pulp/pulp-openapi-generator/pull/58 fixes the bindings issue
15:01 < fao89[m]> I tested here https://github.com/pulp/pulpcore/commit/dd14f3100035b23b0f43bce795a6d2d64016d057
15:02 < ggainey> ah ok
15:02 < ggainey> I'll stop mucking around w/ my PR then :)
15:02 < dkliban[m]> fao89: awesome!
15:03 < dkliban[m]> fao89: i apporved the PR ... ggainey remove that last change from your PR and re-push after fao89 's PR is merged
15:03 < ggainey> fao89++
15:03 < ggainey> dkliban: yeah zacly
15:04 < fao89[m]> merged!
15:05 < ggainey> ...re-pushed, let's see how it goes :)
--- Log closed Thu Jan 06 00:00:04 2022

--- Log opened Thu Jan 06 00:00:04 2022
19:51 < jsherrill> so they released 1.9.0
19:51 < jsherrill> which apparently is breaking us now
19:52 < jsherrill> releaesd it today
19:52 < jsherrill> the error we're getting is:   uninitialized constant Faraday::UploadIO
19:52 < jsherrill> fyi
19:52 < dkliban[m]> yeah ... that's what we are seeing in CI also
19:53 < fao89[m]> yep, faraday managed to break us 2 times in a week
19:53 < fao89[m]> CI was working with 1.8
19:53 < jsherrill> fun fun fun fun 
19:54 < fao89[m]> in 3 hours they released 3 versions
19:54 < fao89[m]> https://github.com/lostisland/faraday/releases
19:55 < jsherrill> someone was having 'fun'
19:55 < dkliban[m]> jsherrill: this just tells me that we need to get on the latest version of openapi generator so we can use newer versions of faraday
19:57 < dkliban[m]> jsherrill: i reproduced the issue you experienced and i believe it's this one https://github.com/OpenAPITools/openapi-generator/issues/11158 ... however, using the master branch of the project doesn't seem to resolve it.
19:57 < dkliban[m]> so i am going to do some java debugging next
20:01 < jsherrill> dkliban[m]: awesome!
20:02 < fao89[m]> hopefully it fixes the faraday issue - https://github.com/pulp/pulp-openapi-generator/pull/59/files
20:03 < jsherrill> fao89[m]: looks right to me, thats what i'm pinning katello to

So uninitialized constant Faraday::UploadIO appears to be the reason to pin to < 1.9. I'll do some further digging.

ekohl commented 2 years ago

I started https://github.com/OpenAPITools/openapi-generator/pull/13127 to resolve that upstream and https://github.com/pulp/pulp-openapi-generator/pull/77 in this repo.