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

Consolidate the loading of modules related to AWS SQS ActiveJob #134

Merged
merged 4 commits into from
Jul 22, 2024

Conversation

kakubin
Copy link
Contributor

@kakubin kakubin commented Jul 20, 2024

partially related: #133

We are using the gem just for processing async job on ActiveJob.
Since we don't need any other functionality of the gem, we would like a way to use it easily like the following.

gem 'aws-sdk-rails', require: false

require 'aws/rails/sqs_active_job' # load modules just about worker

module MyApplication
  class Application < Rails::Application
    config.active_job.queue_adapter = :sqs
  end
end

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license.

@mullermp
Copy link
Contributor

Thanks. This seems fine. Would you be able to also move the associated class config tests out of the configuration spec and into a new spec file that matches where they exist now?

@kakubin kakubin force-pushed the compose_job_modules branch from 68b8e88 to a2a6920 Compare July 21, 2024 03:53
@kakubin
Copy link
Contributor Author

kakubin commented Jul 21, 2024

Would you be able to also move the associated class config tests out of the configuration spec and into a new spec file that matches where they exist now?

Done

@kakubin
Copy link
Contributor Author

kakubin commented Jul 21, 2024

To be honest, since other dependencies like aws-sdk-s3 are not needed in my case, I would like AWS SQS ActiveJob to be separated into a gem and managed similarly to aws-sdk-ruby.

spec/aws/rails/sqs_active_job_spec.rb Outdated Show resolved Hide resolved
@mullermp
Copy link
Contributor

Hm. I wonder if we can drop direct dependency and require them to be loaded instead. I'll think about that.

@kakubin kakubin force-pushed the compose_job_modules branch from a2a6920 to 5428e36 Compare July 21, 2024 14:44
@kakubin kakubin requested a review from mullermp July 21, 2024 14:45
@mullermp
Copy link
Contributor

Thanks for your fix. I'm open to any PRs for making other features optional and not having other SDK gems as direct dependencies.

@mullermp mullermp merged commit 8ff5279 into aws:main Jul 22, 2024
17 checks passed
@kakubin kakubin deleted the compose_job_modules branch July 22, 2024 01:33
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.

2 participants