-
-
Notifications
You must be signed in to change notification settings - Fork 279
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
Environment Load Order #282
Comments
It is very difficult. What kind of options do you want to use in |
But this breaks all forms of env-based config where the environment variables aren't set via an external process. |
We set the queue names from env because our queues change based on environment. |
Maybe the config file should be abandoned altogether for shoryuken-specific options. Or a separate config file for daemonizing options (which I see a lot). |
By the way, where did the other issue come from? We'be been running the previous version of Shoryuken in production for a long time now and have encountered the issue adding a log file option is meant to solve. We just let systemd handle logging redirection from stdout/stderr. As well as handling running processes, etc. |
@waynerobinson actually you can use ENV variables as @h3poteto suggested, just include an initializer in the list of required files like this: Shoryuken.configure_server do |config|
config.aws = {
secret_access_key: ENV["AWS_SECRET_ACCESS_KEY"],
access_key_id: ENV["AWS_ACCESS_KEY_ID"],
region: ENV["AWS_REGION"]
}
end
Shoryuken.configure_client do |config|
config.aws = {
secret_access_key: ENV["AWS_SECRET_ACCESS_KEY"],
access_key_id: ENV["AWS_ACCESS_KEY_ID"],
region: ENV["AWS_REGION"]
}
end WDYT? I still have mixed feeling about aws-sdk initialization and Shoryuken, if we should keep this responsibility in Shoryuken or leave users to set that directly in the |
But it isn't the AWS config, but the Shoryuken config we want to use environment variables in. Especially the queue lists, but also our concurrency count differs in dev/prod to make it easier to diagnose issues. We must be able to put per-environment environment variables in our Shoryuken file or Shoryuken config. Either everything in Shoryuken should be configurable within |
Another thing, aren't the log file and PID file meant to be set as command-line options? |
We can not use environment variables defined by Rails, but we can use normal environment variables, for example Queue option does not need to be kept secret, so we do not need to use environment variables. |
It's got nothing to do with secrecy, it's got to do with different options for various environments. At the very least, if you're going to allow (or insist on) configuration of Shoryuken via the |
@h3poteto another note, the |
@waynerobinson I will try the @waynerobinson @h3poteto regarding the AWS setup, I planning to release a version 3.0.0, which will remove all AWS config from Shoryuken, so we would need to configure Aws using the SDK directly, not through Shoryuken, see this #293. |
@waynerobinson |
I also would be curious to set something up where the production:
... some config here ...
development:
... some other config here ... |
@camkidman The plan is to remove this AWS configuration from Shoryuken #293 and leverage that to the |
@phstc I actually meant the queues and shoryuken options. Here's a different example: production:
concurrency: 25
delay: 25
queues:
- [production_queue]
development:
concurrency: 5
delay: 5
queues:
- [development_queue] |
@camkidman got it. I'm not sure if we can go with fixed environments like that, it can be very specific for each app. One option is to have
queues:
<% if production? %>
- [production_queue]
<% else %>
- [development_queue]
<% end %> |
I did try using |
Note: we are using dotenv in dev and this is working: queues:
- [<%= ENV.fetch('DEFAULT_JOB_QUEUE_NAME') %>, 2] but you have to invoke it like: |
Unfortunately it doesn't because it needs to know what the Rails
environment is as it starts.
…On Tue, 10 Jan 2017 at 8:17 am, TJ Singleton ***@***.***> wrote:
Note: we are using dotenv in dev and this is working:
queues:
- [<%= ENV.fetch('DEFAULT_JOB_QUEUE_NAME') %>, 2]
but you have to invoke it like: dotenv shoryuken -R -C
config/shoryuken.yml which is setting up the environment before exec'ing
shoryuken. I'm not sure figaro supports such a wrapper, a scan of it's
source seems that it is tied to rails.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#282 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAG4pt8gUXDK200mjrddnaFcdSIX8EVVks5rQqPpgaJpZM4LIuab>
.
|
So looks like if you want to use |
@Kitton would setting |
@phstc Sorry, I didn't express myself correctly. |
@Kitton are your queues fixed, like a set of queue names in staging, another set in production or do they change dynamically? If they are fixed, you may use the |
@Kitton I'm not familiar with Figaro, but checking their docs, it seems that they have also |
FWIW, I use a workaround similar to the one suggested by @phstc , but still looking for a better solution. |
@h3poteto I've naively moved |
I think it had to do with specifying the log file via an environment variable rather than the command-line option. Which I would suggest is an anti-pattern anyway as if you care this much about log files you should either specify it on the command-line during whatever startup script you're using, or just let the higher-level daemonizer do the logging (which is what we do). |
@waynerobinson I couldn't reproduce it, I tried to start it with |
@waynerobinson sorry for taking long on this. I hope we can fix this soon. |
@phstc I read #318. #219 says to daemonize before loading Rails, so call So, when calling |
@h3poteto is right - #318 will reintroduce the problems that #219 was intended to solve. In case it isn't clear, there is a dependency cycle in the boot process:
We can't have all three of these things at the same time. No matter how we rearrange the order of operations, we have to give up one of them. |
I'm not sure why the third is necessary when there is a command-line option for it and logging should largely be done to STDOUT/STDERR anyway and let the thing running it redirect appropriately. |
@eugeneius so Sidekiq does not support loading |
ok, I've just confirmed that. Sidekiq does not seem to allow Rails in the So the options are:
|
@h3poteto @eugeneius @waynerobinson ☝️ any thoughts? |
When it comes down to it, this was to solve one core problem: How do you programmatically set the queues after Rails has loaded. Sidekiq doesn't have this problem because it's super easy to keep a different dev & production Redis server around and configure it that way. But with SQS, every developer needs their own SQS queue and, since the only place these can be set is in the It's equally unreasonable to expect that the *only * configuration gem that is supposed by Shoryuken is Dotenv (and only because you can basically use it to modify the environment before it executes something else, rather than the currently running ruby process). I vote to get rid of the If I can set queues somewhere like we setup the AWS config, my personal issues with this go away. |
@waynerobinson can't you use I used to use
I don't see the point of getting rid of it. I think there are other options for you problem as suggested above or just add something to Shoryuken to allow adding queues at runtime. |
@waynerobinson BTW, this: # shoryuken_setup.rb
Shoryuken.configure_server do |config|
config.queues << 'default'
# you can use Rails in here, or anything initialized by Rails
end bundle exec shoryuken -R -r ./shoryuken_setup.rb worked just fine for me. It loaded Rails, then added the queue Does that work for you? |
It hasn't previously. Or however I attempted to do it via assumed API. How do you specify priority with that syntax? |
@waynerobinson that's a great question. It would require a change in Shoryuken to make it easier, something like this Shoryuken.configure_server do |config|
[
['default', 5],
['high', 10]
].each do |queue_name, priority|
priority.times { config.queues << queue_name }
end
end Let me know if you were able to test it and if that works for you. So we can wrap that in a PR/Wiki. |
I will check it out. |
I agree with this opinion.
Therefore, I think it is very good solution. |
@h3poteto is it because for dev environments? I'm wondering if having something like the ActiveJob Inline Adapter natively in Shoryuken would help. Causing |
@phstc Oh, it looks very useful. But, I assumed other cases which is not only development. In Sidekiq: In Shoryuken: Therefore, I don't have to write environment name in queue and switch queue in Sidekiq, but it is required in Shoryuken. |
Fixed by #322 |
We use Rails and our config data is loaded via Figaro into the environment.
In the latest version of Shoryuken, the
shoryuken.yml
file gets parsed/loaded before the Rails environment loads and Figaro has a chance to inject the extra options intoENV
.I haven't tested this with Dotenv, but I suspect it would have a similar problem if you were using environment variables in a
shoryuken.yml
file.Can the
Shoryuken::EnvironmentLoader#load_rails
file be safely moved to the top ofShoryuken::EnvironmentLoader#setup_options
?The text was updated successfully, but these errors were encountered: