Giter VIP home page Giter VIP logo

Comments (12)

benhamill avatar benhamill commented on June 1, 2024

The method_missing implementation is there to support dynamic search method names. Those kind of method names are falling out of favor, I think, in Rails, which I believe to be the inspiration for them in the first place. I don't want to just rip them out, however, without discussing among the other maintainers (I, personally, have never found them to be much use).

However, I did just change method_missing a bit in master, and I wonder if you could see if it fixed this issue for you? Or if you could help me reproduce, I could test it myself.

Also: Sorry for the long response time.

from textacular.

DrTom avatar DrTom commented on June 1, 2024

Yes, I am aware that the dynamic finders that method_missing are falling out of favor. Imho named arguments turn out to be more readable than find_by_this_and_than_and_y_and_z. Without the trouble that overwriting method missing brings along.

At any rate. I tried the current commit of textacular on github and our project still fails when running the migrations with it. I created a branch from the failing commit if you want too look in to it: https://github.com/zhdk/madek/tree/textacular
The config/database_developer_example.yml shows an example how we connect. As mentioned, the failing task is rake db:migrate.

from textacular.

benhamill avatar benhamill commented on June 1, 2024

Hmm. I checked it out and had a look at the stack trace. It didn't look like textacular was specifically implicated:

PG::Error: ERROR:  current transaction is aborted, commands ignored until end of transaction block
: CREATE TABLE "zencoder_jobs" ("id" uuid NOT NULL, "media_file_id" integer NOT NULL, "zencoder_id" integer, "comment" text, "state" character varying(255) DEFAULT 'initialized' NOT NULL, "error" text, "notification" text, "request" text, "response" text, "created_at" timestamp NOT NULL, "updated_at" timestamp NOT NULL) /home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rack-mini-profiler-0.1.26/Ruby/lib/patches/sql_patches.rb:155:in `exec'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rack-mini-profiler-0.1.26/Ruby/lib/patches/sql_patches.rb:155:in `async_exec'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/connection_adapters/postgresql_adapter.rb:650:in `block in execute'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract_adapter.rb:280:in `block in log'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activesupport-3.2.13/lib/active_support/notifications/instrumenter.rb:20:in `instrument'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract_adapter.rb:275:in `log'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/connection_adapters/postgresql_adapter.rb:649:in `execute'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/schema_statements.rb:170:in `create_table'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/foreigner-1.4.1/lib/foreigner/connection_adapters/abstract/schema_statements.rb:14:in `create_table'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/migration.rb:466:in `block in method_missing'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/migration.rb:438:in `block in say_with_time'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/2.0.0/benchmark.rb:281:in `measure'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/migration.rb:438:in `say_with_time'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/migration.rb:458:in `method_missing'
/home/ben/src/madek/db/migrate/20130205144924_create_zencoder_jobs.rb:16:in `up'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/migration.rb:410:in `block (2 levels) in migrate'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/2.0.0/benchmark.rb:281:in `measure'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/migration.rb:410:in `block in migrate'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/connection_pool.rb:129:in `with_connection'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/migration.rb:389:in `migrate'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/migration.rb:528:in `migrate'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/migration.rb:720:in `block (2 levels) in migrate'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/migration.rb:775:in `call'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/migration.rb:775:in `block in ddl_transaction'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/connection_adapters/abstract/database_statements.rb:192:in `transaction'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/transactions.rb:208:in `transaction'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/migration.rb:775:in `ddl_transaction'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/migration.rb:719:in `block in migrate'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/migration.rb:700:in `each'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/migration.rb:700:in `migrate'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/migration.rb:570:in `up'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/migration.rb:551:in `migrate'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/activerecord-3.2.13/lib/active_record/railties/databases.rake:193:in `block (2 levels) in <top (required)>'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rake-10.0.4/lib/rake/task.rb:246:in `call'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rake-10.0.4/lib/rake/task.rb:246:in `block in execute'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rake-10.0.4/lib/rake/task.rb:241:in `each'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rake-10.0.4/lib/rake/task.rb:241:in `execute'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rake-10.0.4/lib/rake/task.rb:184:in `block in invoke_with_call_chain'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/2.0.0/monitor.rb:211:in `mon_synchronize'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rake-10.0.4/lib/rake/task.rb:177:in `invoke_with_call_chain'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rake-10.0.4/lib/rake/task.rb:170:in `invoke'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rake-10.0.4/lib/rake/application.rb:143:in `invoke_task'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rake-10.0.4/lib/rake/application.rb:101:in `block (2 levels) in top_level'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rake-10.0.4/lib/rake/application.rb:101:in `each'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rake-10.0.4/lib/rake/application.rb:101:in `block in top_level'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rake-10.0.4/lib/rake/application.rb:110:in `run_with_threads'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rake-10.0.4/lib/rake/application.rb:95:in `top_level'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rake-10.0.4/lib/rake/application.rb:73:in `block in run'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rake-10.0.4/lib/rake/application.rb:160:in `standard_exception_handling'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rake-10.0.4/lib/rake/application.rb:70:in `run'
/home/ben/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/rake-10.0.4/bin/rake:33:in `<top (required)>'
./bin/rake:16:in `load'
./bin/rake:16:in `<main>'
Tasks: TOP => db:migrate

However, if removing this gem fixes it, then that's telling. I'm not sure how to go about figuring out how to fix it, so I'll open a new Issue specifically to discuss removing the dynamic method feature. Obviously, that would require a major version bump, too, so we're not looking at the next release for this.

from textacular.

gregmolnar avatar gregmolnar commented on June 1, 2024

@DrTom I cloned that repo and ran the migrations with the master of textacular without any issue.
I was on the wrong branch. Testing it again.

from textacular.

benhamill avatar benhamill commented on June 1, 2024

Oh. @DrTom: What version of Ruby are you using? That might matter, too.

from textacular.

gregmolnar avatar gregmolnar commented on June 1, 2024

It is failing for me too. I use the same patchlevel of ruby 2 as you. It is a very weird error. I will try to find where it is originated from.

from textacular.

gregmolnar avatar gregmolnar commented on June 1, 2024

I tried to get to the bottom of this but no luck so far. It brakes at the override of respond_to?

from textacular.

gregmolnar avatar gregmolnar commented on June 1, 2024

I might be wrong but at this line https://github.com/textacular/textacular/blob/master/lib/textacular.rb#L56 should we check if it is an ActiveRecord::Base instance and return super if not? If change the code like that than the migrations are not breaking and the dynamic search methods are working too. Although the spec for this is broken but I guess it is an issue with the test.
This what I would put in that line instead:

return super if self != ActiveRecord::Base

@benhamill do you think this would be a good fix? I can commit the changes if you think it is ok.

from textacular.

benhamill avatar benhamill commented on June 1, 2024

That abstract_class? call went in 2 days ago in this commit. And this issue predates it...

Ignoring the test for the moment, if the class in question is ActiveRecord::Base, then we don't want to check if it's a dynamic helper method, because AR::Base doesn't have columns out of which to build helpers (or, to bring the test back in, any other abstract class).

But I feel like I might just be misunderstanding something about what you're saying.

from textacular.

gregmolnar avatar gregmolnar commented on June 1, 2024

You are right it was late and I haven't think it through. Oddly enough if I do the change above to textacular in DrTom's app the migrations are working and even the dynamic helpers. Not sure how :) .
So I guess there id another gem which overwrites respond_to? in that app which conflicts with textacular. I will have free time tomorrow and I will investigate this further.

from textacular.

DrTom avatar DrTom commented on June 1, 2024

respond_to? is overwritten at least 55 times from various gems which madek uses (I searched with def\s*respond_to\? but there are more subtle ways to overwrite things in ruby, so I might have missed some).

I eliminated a few suspicious gems but the migration still fails. So, we know that there is some unfortunate interaction between textacular and some other gem that overwrites respond_to? as well.

I will use my patched version of textacular without the overwritten method_missing and respond_to? for the time being. I will prepare Madek for Rails 4 in the next month and also remove some further gems I want to get rid of anyways. I can test the original version of textacular after the refactoring again.

from textacular.

benhamill avatar benhamill commented on June 1, 2024

I'm gonna go ahead and close this. I just merged #16, which deprecates the dynamic helpers. In the next major version bump, we'll rip 'em out.

from textacular.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.