Move Faraday middlewares register to an initializer
🐼 Context
In !50750 (merged), the Container Registry client observability has been improved by using a custom Faraday middleware.
This middleware is registered using this class. Reasons explained here.
For a reason I don't clearly see, this commit broke that because ::Gitlab::Faraday
is not eager loaded anymore.
To illustrate this point, here is what happens before bc05f258:
[1] pry(main)> Faraday::Request.lookup_middleware(:gitlab_error_callback)
=> Gitlab::Faraday::ErrorCallback
Our custom middleware is properly registered and Faraday can look it up.
Here is currently the situation on master
:
Faraday::Request.lookup_middleware(:gitlab_error_callback)
Faraday::Error: :gitlab_error_callback is not registered on Faraday::Request
from /Users/david/.asdf/installs/ruby/2.7.2/lib/ruby/gems/2.7.0/gems/faraday-1.0.1/lib/faraday/middleware_registry.rb:91:in `lookup_middleware'
Note that this does not affect production environments in any way, as all the classes are eager loaded on these.
I'm not sure why bc05f258 breaks the class loading. This commit seems to change how "models" are loaded and our custom middleware is loaded from a class that is in lib
. lib
should be eager loaded all the times. I'm not sure what is happening
🔭 What does this MR do?
- Register the Faraday middleware from an initializer file instead of a class file.
I don't think that a changelog entry is need here. This is Stuff that should Just Work .
🖼 Screenshots (strongly suggested)
With this MR:
Faraday::Request.lookup_middleware(:gitlab_error_callback)
=> Gitlab::Faraday::ErrorCallback
✅ Does this MR meet the acceptance criteria?
🔩 Conformity
- [-] Changelog entry
- [-] Documentation (if required)
-
Code review guidelines -
Merge request performance guidelines -
Style guides - [-] Database guides
- [-] Separation of EE specific content
🎠 Availability and Testing
- [-] Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
- [-] Tested in all supported browsers
- [-] Informed Infrastructure department of a default or new setting change, if applicable per definition of done
🔒 Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
- [-] Label as security and @ mention
@gitlab-com/gl-security/appsec
- [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
- [-] Security reports checked/validated by a reviewer from the AppSec team