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

Provide an xml parser engine for aws sdk core version 2 #781

Closed
wants to merge 2 commits into from

Conversation

jpawlyn
Copy link
Contributor

@jpawlyn jpawlyn commented Jan 21, 2025

otherwise we see the error:

Failure/Error: self.url  = client.get_queue_url(queue_name: name).queue_url

      NoMethodError:
        undefined method `new' for false

when running tests

otherwise we see the error:

Failure/Error: self.url  = client.get_queue_url(queue_name: name).queue_url

      NoMethodError:
        undefined method `new' for false

when running tests
@jpawlyn
Copy link
Contributor Author

jpawlyn commented Jan 21, 2025

We are also seeing a failure for the Rails 4.2 test run. Perhaps it makes sense just to remove this from the test matrix ?

@phstc
Copy link
Collaborator

phstc commented Jan 21, 2025

@jpawlyn thank you! tests are in progress.

Rails 4.2 test run

Can it be fixed? I wonder if we remove it and there are indeed people using 4.2.

@jpawlyn
Copy link
Contributor Author

jpawlyn commented Jan 22, 2025

Can it be fixed? I wonder if we remove it and there are indeed people using 4.2.

4.2 was first released in 2014 and stopping support for it perhaps makes sense for ease of maintenance @phstc ?

@phstc
Copy link
Collaborator

phstc commented Jan 22, 2025

@jpawlyn good call, that makes sense. Can you remove it then?

We will probably need to release a major version given the breaking change, WDYT?

@jpawlyn
Copy link
Contributor Author

jpawlyn commented Jan 23, 2025

@jpawlyn good call, that makes sense. Can you remove it then?

We will probably need to release a major version given the breaking change, WDYT?

I've removed Rails 4.2 from the matrix. I guess the change is not actually a breaking one since all we are doing is removing Rails 4.2 from the test matrix and there is no minimum Rails version specified in the gemspec.

@phstc
Copy link
Collaborator

phstc commented Jan 23, 2025

I guess the change is not actually a breaking one

@jpawlyn I thought it was a breaking change because the change was breaking something on Rails 4.2.

@jpawlyn
Copy link
Contributor Author

jpawlyn commented Jan 23, 2025

@jpawlyn I thought it was a breaking change because the change was breaking something on Rails 4.2.

Can you clarify @phstc what the change is ? Is there perhaps a merged PR that may have caused this test failure ?

@mensfeld
Copy link
Contributor

@phstc WDYT about aligning the supported versions with Rails supported versions + 6 months? I do this for Karafka and works well. There needs to be a bit of backpressure and anyhow it seems AWS gem releases and API changes create some already.

@phstc
Copy link
Collaborator

phstc commented Jan 28, 2025

@mensfeld I think it's a solid idea. Sidekiq for instance is Rails 7.0+

@mensfeld
Copy link
Contributor

closing this one as after my todays changes there's nothing here to merge that would make sense.

@mensfeld mensfeld closed this Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants