Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the rails 7.1+ serialize interface #23253

Merged

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented Nov 1, 2024

Type was changed from positional to a keyword argument 'type' in 7.1. The positional argument is removed in rails 7.2.

We have a helper bridge module prepending support for either kwargs or positional arguments passed to serialize which converts to positional for rails 7.0 and kwargs for 7.1+. This was added and automatically loaded in the rails app via the engine in schema:
ManageIQ/manageiq-schema#756

Extracted from #23225

jrafanie added a commit to jrafanie/manageiq-consumption that referenced this pull request Nov 1, 2024
Type was changed from positional to a keyword argument 'type' in 7.1.
The positional argument is removed in rails 7.2.

We have a helper bridge module prepending support for either kwargs
or positional arguments passed to serialize which converts to positional
for rails 7.0 and kwargs for 7.1+.  This was added and automatically
loaded in the rails app via the engine in schema:
ManageIQ/manageiq-schema#756

Similar change as core: ManageIQ/manageiq#23253
@jrafanie
Copy link
Member Author

jrafanie commented Nov 1, 2024

Ok, it's failing on a real issue...will mark as wip... interestingly service_mixin referencing service to ensure an object can't be associated with more than one service... note, service is an alias_attribute of parent... this is one of the "problems" in rails 7.1 as alias_attribute doesn't support non-attributes such as parent (an association).

@jrafanie jrafanie changed the title Use the rails 7.1+ serialize interface [WIP] Use the rails 7.1+ serialize interface Nov 1, 2024
@jrafanie jrafanie added the wip label Nov 1, 2024
@kbrock
Copy link
Member

kbrock commented Nov 11, 2024

Think these are legit / not too hard to fix:

spec/models/vm_spec.rb:288
spec/models/service_spec.rb:400
spec/models/service_spec.rb:806

assuming it is because a different exception is raised

Type was changed from positional to a keyword argument 'type' in 7.1.
The positional argument is removed in rails 7.2.

We have a helper bridge module prepending support for either kwargs
or positional arguments passed to serialize which converts to positional
for rails 7.0 and kwargs for 7.1+.  This was added and automatically
loaded in the rails app via the engine in schema:
ManageIQ/manageiq-schema#756
@jrafanie jrafanie force-pushed the serialize-with-kwargs-like-71-wants branch from 96a76f6 to 5cec895 Compare November 19, 2024 19:05
@jrafanie jrafanie changed the title [WIP] Use the rails 7.1+ serialize interface Use the rails 7.1+ serialize interface Nov 19, 2024
@jrafanie jrafanie removed the wip label Nov 19, 2024
@kbrock kbrock merged commit 52cf57e into ManageIQ:master Nov 19, 2024
8 checks passed
@kbrock kbrock deleted the serialize-with-kwargs-like-71-wants branch November 19, 2024 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants