Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Add compatibility with Ruby 3#992

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@indirect
Copy link
Contributor

Since Ruby 3 changed the way keyword arguments work, Rails now only accepts keyword arguments forfind_in_batches. Unfortunately, themethod_missing implementation onElasticsearch::Model::Proxy doesn't proxy keyword arguments, so Rails now explodes. The error is triggered by callingModel.import, and looks something like this:

ArgumentError: wrong number of arguments (given 1, expected 0)bundle/gems/activerecord-6.1.3.1/lib/active_record/relation/batches.rb:128:in 'find_in_batches'

The fix is to forward not just positional arguments, but also keyword arguments. This PR adds a test (failing before the patch), and a patch to forward keyword arguments.

Fixes#989,#977.

@cla-checker-service
Copy link

cla-checker-servicebot commentedApr 26, 2021•
edited
Loading

💚 CLA has been signed

@indirect
Copy link
ContributorAuthor

I have now read and signed the contributor agreement.

@indirect
Copy link
ContributorAuthor

I've updated the implementation to work on Ruby 2.4 through 3.0, based onthis ruby-core blog post, which should keep this change backwards-compatible.

jihodge reacted with heart emoji

momocus added a commit to momocus/sakazuki that referenced this pull requestJul 3, 2021
momocus added a commit to momocus/sakazuki that referenced this pull requestJul 3, 2021
momocus added a commit to momocus/sakazuki that referenced this pull requestJul 3, 2021
momocus added a commit to momocus/sakazuki that referenced this pull requestJul 3, 2021
momocus added a commit to momocus/sakazuki that referenced this pull requestJul 3, 2021
@picandocodigo
Copy link
Member

Hi@indirect,
Thank you very much for this contribution! I'm working on a new release and I'd like to include this change. However, could you please restorethis line? We need to keep the license header on every source code file.

Thanks!

when the proxy fails to forward keywoard arguments, the error formethods that only accept keyword arguments looks like this:ArgumentError:  wrong number of arguments (given 1, expected 0)
this resolves a bug with Rails 6.1 on Ruby 3, where keyword arguments are not forwarded and this causes an ArgumentError
since Ruby 1.9.2, about 12 years ago, you're supposed to define respond_to_missing? if you also define method_missing. there's more explanation in this blog post by a ruby committer in 2010:http://blog.marc-andre.ca/2010/11/15/methodmissing-politely/
@indirectindirectforce-pushed theproxy-argument-error-fix branch fromd35acf3 tod9c062cCompareAugust 12, 2021 08:12
@indirect
Copy link
ContributorAuthor

Whoops, I must have accidentally deleted the first line when I opened the file and not noticed. Fixed.

@therrick
Copy link

Any outlook on merging this?

fabriciofreitag reacted with thumbs up emoji

@picandocodigopicandocodigo merged commitd12d812 intoelastic:masterSep 7, 2021
@tiendo1011
Copy link

@picandocodigo can you release a new version that includes this fix?
The newest one (7.2.0) is released before this fix is merged.
Thank you

@sebfie
Copy link

I also need this fix ! Please can you release a new version?

@jonnynux
Copy link

I upvote for releasing

@picandocodigo
Copy link
Member

Version 7.2.1 has been released with these changes.

tiendo1011, bockets, and alejanderl reacted with hooray emoji

@mojobiri
Copy link

mojobiri commentedMay 27, 2022•
edited
Loading

Would it be possible to add this patch to the 6.x version? We are using Elasticsearch 6 and would love to have Ruby 3 support in 6.1.2 too. Right now we have to monkeypatch.

greybutton and andrew-david-smith reacted with thumbs up emoji

@dersnek
Copy link

Would it be possible to add this patch to the 6.x version? We are using Elasticsearch 6 and would love to have Ruby 3 support in 6.1.2 too. Right now we have to monkeypatch.

#1053

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Rails Application Template 03-expert.rb - ArgumentError: wrong number of arguments (given 1, expected 0)

8 participants

@indirect@picandocodigo@therrick@tiendo1011@sebfie@jonnynux@mojobiri@dersnek
[8]ページ先頭
©2009-2025 Movatter.jp