Closed n-rodriguez closed 10 months ago
jnunemaker
commented
10 months ago Hi @n-rodriguez thanks for reporting. This is odd. Redis shouldn't matter here. It's just requiring a file in flipper that defines a few helper methods.
You are on rails 7.1.2 and ruby 3.2.2, correct? Anything else unique or different about your setup that you can think of?
@bkeepers I did a blame on that and it was me that added it. I haven't done a lot in the engine though, anything in there look wrong or odd?
n-rodriguez
commented
10 months ago Hi @n-rodriguez thanks for reporting. This is odd. Redis shouldn't matter here
I think it's the opposite: I use redis, active record shouldn't matter here.
jnunemaker
commented
10 months ago @n-rodriguez the reason it shouldn't matter is this is about extending active record to automatically include flipper_id and flipper_properties for each active record model. That's why I said it shouldn't matter. It has nothing to do with redis specifically or your flipper adapter. It's just generic extension of active record. Additionally, it shouldn't matter because the file doesn't require active record or actually do any of the extending.
Can you set your Gemfile to this:
gem "flipper", {git: "https://github.com/jnunemaker/flipper", branch: "delayed-require"}And let me know if that works? If so, I can cut a patch release for ya.
n-rodriguez
commented
10 months ago Can you set your Gemfile to this:
gem "flipper", {git: "https://github.com/jnunemaker/flipper", branch: "delayed-require"}And let me know if that works? If so, I can cut a patch release for ya.
Done, still don't work :
<internal:/Users/nicolas/.asdf/installs/ruby/3.2.2/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:38:in `require': cannot load such file -- flipper/model/active_record (LoadError)
from <internal:/Users/nicolas/.asdf/installs/ruby/3.2.2/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:38:in `require'
from /Users/nicolas/PROJECTS/CONCERTO/concerto/.bundle/ruby/3.2.0/gems/bootsnap-1.17.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:17:in `require'
from /Users/nicolas/PROJECTS/CONCERTO/concerto/.bundle/ruby/3.2.0/gems/zeitwerk-2.6.12/lib/zeitwerk/kernel.rb:38:in `require'
from /Users/nicolas/PROJECTS/CONCERTO/concerto/.bundle/ruby/3.2.0/gems/flipper-1.1.0/lib/flipper/engine.rb:19:in `block (2 levels) in <class:Engine>'Delaying require "flipper/model/active_record" can't work because there's no flipper/model/active_record in gem tree :
nicolas@MacBook-Pro-de-Nicolas:~/PROJECTS/CONCERTO/concerto/.bundle/ruby/3.2.0/gems/flipper-1.1.0$ tree lib/
lib/
├── flipper
│ ├── actor.rb
│ ├── adapter.rb
│ ├── adapter_builder.rb
│ ├── adapters
│ │ ├── dual_write.rb
│ │ ├── failover.rb
│ │ ├── failsafe.rb
│ │ ├── http
│ │ │ ├── client.rb
│ │ │ └── error.rb
│ │ ├── http.rb
│ │ ├── instrumented.rb
│ │ ├── memoizable.rb
│ │ ├── memory.rb
│ │ ├── operation_logger.rb
│ │ ├── poll
│ │ │ └── poller.rb
│ │ ├── poll.rb
│ │ ├── pstore.rb
│ │ ├── read_only.rb
│ │ ├── strict.rb
│ │ ├── sync
│ │ │ ├── feature_synchronizer.rb
│ │ │ ├── interval_synchronizer.rb
│ │ │ └── synchronizer.rb
│ │ └── sync.rb
│ ├── cloud
│ │ ├── configuration.rb
│ │ ├── dsl.rb
│ │ ├── message_verifier.rb
│ │ ├── middleware.rb
│ │ ├── routes.rb
│ │ ├── telemetry
│ │ │ ├── backoff_policy.rb
│ │ │ ├── instrumenter.rb
│ │ │ ├── metric.rb
│ │ │ ├── metric_storage.rb
│ │ │ └── submitter.rb
│ │ └── telemetry.rb
│ ├── cloud.rb
│ ├── configuration.rb
│ ├── dsl.rb
│ ├── engine.rb
│ ├── errors.rb
│ ├── export.rb
│ ├── exporter.rb
│ ├── exporters
│ │ └── json
│ │ ├── export.rb
│ │ └── v1.rb
│ ├── expression
│ │ ├── builder.rb
│ │ └── constant.rb
│ ├── expression.rb
│ ├── expressions
│ │ ├── all.rb
│ │ ├── any.rb
│ │ ├── boolean.rb
│ │ ├── comparable.rb
│ │ ├── duration.rb
│ │ ├── equal.rb
│ │ ├── greater_than.rb
│ │ ├── greater_than_or_equal_to.rb
│ │ ├── less_than.rb
│ │ ├── less_than_or_equal_to.rb
│ │ ├── not_equal.rb
│ │ ├── now.rb
│ │ ├── number.rb
│ │ ├── percentage.rb
│ │ ├── percentage_of_actors.rb
│ │ ├── property.rb
│ │ ├── random.rb
│ │ ├── string.rb
│ │ └── time.rb
│ ├── feature.rb
│ ├── feature_check_context.rb
│ ├── gate.rb
│ ├── gate_values.rb
│ ├── gates
│ │ ├── actor.rb
│ │ ├── boolean.rb
│ │ ├── expression.rb
│ │ ├── group.rb
│ │ ├── percentage_of_actors.rb
│ │ └── percentage_of_time.rb
│ ├── identifier.rb
│ ├── instrumentation
│ │ ├── log_subscriber.rb
│ │ ├── statsd.rb
│ │ ├── statsd_subscriber.rb
│ │ └── subscriber.rb
│ ├── instrumenters
│ │ ├── memory.rb
│ │ └── noop.rb
│ ├── metadata.rb
│ ├── middleware
│ │ ├── memoizer.rb
│ │ └── setup_env.rb
│ ├── poller.rb
│ ├── registry.rb
│ ├── serializers
│ │ ├── gzip.rb
│ │ └── json.rb
│ ├── spec
│ │ └── shared_adapter_specs.rb
│ ├── test
│ │ └── shared_adapter_test.rb
│ ├── type.rb
│ ├── typecast.rb
│ ├── types
│ │ ├── actor.rb
│ │ ├── boolean.rb
│ │ ├── group.rb
│ │ ├── percentage.rb
│ │ ├── percentage_of_actors.rb
│ │ └── percentage_of_time.rb
│ └── version.rb
└── flipper.rb
20 directories, 100 filesOk, that's interesting. Definitely the problem. I'm sure I can reproduce and get this fixed.
n-rodriguez
commented
10 months ago this is about extending active record to automatically include flipper_id and flipper_properties for each active record model
fine, but moving from Flipper::Identifier to Flipper::Model::ActiveRecord seems to be a bad idea for people who don't use flipper-active_record.
jnunemaker
commented
10 months ago @n-rodriguez the active record gem spec is grabbing that. That's the issue. https://github.com/flippercloud/flipper/blob/acab7e4eb21048213897173e3571872993fb8101/flipper-active_record.gemspec#L6
So it was both related and unrelated. Haha. I'll get it fixed up.
jnunemaker
commented
10 months ago @n-rodriguez can you change the branch to fix-ar-ext-require and see if that works for you?
jnunemaker
commented
10 months ago fine, but moving from Flipper::Identifier to Flipper::Model::ActiveRecord seems to be a bad idea for people who don't use flipper-active_record.
Yeah, it will no longer do anything unless you use active record in the branch I just shared. Flipper::Identifier still exists for those not using AR as adapter or for models.
jnunemaker
commented
10 months ago I verified locally with a new rails app and flipper-redis that 1.1.0 caused missing require and locking to main fixed it. I'm going to release 1.1.1 but let me know if you have further issues.
Lastly, if you don't mind, I would love to know what you are using flipper for as well and how it is working out for you (what went well, what was hard other than what you mentioned here).
If you aren't comfortable leaving that information here in a public comment, feel free to email me directly john@fewerandfaster.com. I'm just always curious. :)
n-rodriguez
commented
10 months ago It works! thank you!
I'm using Flipper in a CRM application developed by myself for a small real estate company.
Like many others, I use feature flags for rolling out new features, conducting experiments and activating/deactivating features when a service we use experiences an outage.
jnunemaker
commented
10 months ago Awesome! Thanks for the feedback. Always curious. :)
Hi there!
I've updated flipper to version 1.1.0 and now got this error :
It comes from https://github.com/flippercloud/flipper/blob/main/lib/flipper/engine.rb#L17.
Note: I use the redis adapter.
Thank you!