mount sidekiq dashboard #2772

ViditChitkara · 2018-06-05T19:43:25Z

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
code is in uniquely-named feature branch and has no merge conflicts
PR is descriptively titled
PR body includes fixes #0000-style reference to original issue #
ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

We have a loose schedule of reviewing and pulling in changes every Tuesday and Friday, and publishing changes on Fridays.

Thanks!

ViditChitkara · 2018-06-05T19:48:00Z

Hi @jywarren , sidekiq ran perfectly on my local machine. On production we could use sidekiq dashboard (could be used to verify if sidekiq is working). Would be adding a cron job for digest mails in this pr.

jywarren · 2018-06-05T19:51:32Z

OH cool -- anything we need to do to secure the sidekiq dashboard?

ViditChitkara · 2018-06-05T20:08:41Z

There maybe some authentication feature or something, looking into it.

plotsbot · 2018-06-05T20:10:46Z

	1 Warning
⚠️	It looks like you merged from master in this pull request. Please rebase to get rid of the merge commits – you may want to rewind the master branch and rebase instead of merging in from master, which can cause problems when accepting new code!

	2 Messages
📖	@ViditChitkara Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution! And don’t be discouraged if you see errors – we’re here to help.
📖	It looks like you haven’t marked all the checkboxes. Help us review and accept your suggested changes by going through the steps one by one. If it is still a ‘Work in progresss’, please include ‘[WIP]’ in the title.

Generated by 🚫 Danger

namangupta01 · 2018-06-05T21:43:16Z

@ViditChitkara Its good that you have added password protection on this route so that only specific person could see it.

namangupta01 · 2018-06-05T21:46:00Z

config/schedule.rb

-	puts Dir.pwd
-	command "date -u" #This will print utc time every 1 min in log/cron_log.log file
-end
+# every 1.minutes do


I think we should not comment this? The only thing it is doing here is printing the time every one minute so that we can make sure that it is running. What do you say?

Oh I was testing something, forgot to uncomment it. Will do!

ViditChitkara · 2018-06-06T18:42:52Z

@jywarren , @icarito Some tests are failing :-
There was a test error at: Failure: test_digest_emails(Minitest::Result) [/usr/local/bundle/gems/redis-4.0.1/lib/redis/client.rb:344]: Redis::CannotConnectError: Error connecting to Redis on 127.0.0.1:6379 (Errno::ECONNREFUSED)
Is there something we need for travis tests to pass, regarding redis in docker-compose.yml?
Also, is there any other way with which we could confirm if docker-compose.yml was built successfully and redis is running?

jywarren · 2018-06-23T17:41:34Z

@icarito this coming week if you have a moment to look at this, we'd love your help - thanks!

jywarren · 2018-06-23T17:42:15Z

Vidit, I think it maybe an issue of routing between containers. I'm not an expert on this but you may be able to search stack overflow a bit to try to resolve it?

ViditChitkara · 2018-06-23T18:13:53Z

Yep, I'll try and search it. It appears to be just a small tweak, but we have to search deeper for this. I would be needing @icarito 's help. Going to do this, coming week. Side by side would be working on the 'like with multiple reactions' follow-ups. Currently we are not showing usernames of those who have liked a particular comment, my next pr would be for this. Does this sound good?

jywarren · 2018-06-23T19:30:38Z

Yes, great!

…

On Sat, Jun 23, 2018, 11:14 AM Vidit ***@***.***> wrote: Yep, I'll try and search it. It appears to be just a small tweak, but we have to search deeper for this. I would be needing @icarito <https://github.com/icarito> 's help. Going to do this, coming week. Side by side would be working on the 'like with multiple reactions' follow-ups. Currently we are not showing usernames of those who have liked a particular comment, my next pr would be for this. Does this sound good? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2772 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJzyQhN5QdJOz40VOJN51a-dRp4fRks5t_oVigaJpZM4UbcGm> .

icarito · 2018-06-23T20:50:36Z

Hi vidit sorry I didn't get to this yet but will do first thing Monday. Thanks for your patience. Have a great weekend! El sábado, 23 de junio de 2018, Jeffrey Warren <[email protected]> escribió:

…

Yes, great! On Sat, Jun 23, 2018, 11:14 AM Vidit ***@***.***> wrote: > Yep, I'll try and search it. It appears to be just a small tweak, but we > have to search deeper for this. I would be needing @icarito > <https://github.com/icarito> 's help. Going to do this, coming week. Side > by side would be working on the 'like with multiple reactions' follow-ups. > Currently we are not showing usernames of those who have liked a particular > comment, my next pr would be for this. Does this sound good? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#2772 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ AABfJzyQhN5QdJOz40VOJN51a-dRp4fRks5t_oVigaJpZM4UbcGm> > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2772 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMMS-9Y8oaF3H1HQHXElW_RKI912y_Sks5t_pdggaJpZM4UbcGm> .

-- Sebastian Silva http://somosazucar.org/

icarito · 2018-06-25T07:56:28Z

Hi @ViditChitkara please review this patch ViditChitkara#1 it produced a different error apparently unrelated to Redis. Hope that helps!

icarito · 2018-06-25T08:09:59Z

ERROR["test_digest_emails", Minitest::Result, 55.16748498300001]
 test_digest_emails#Minitest::Result (55.17s)
NoMethodError:         NoMethodError: undefined method `perform_later' for DigestMailJob:Class
            test/functional/users_controller_test.rb:254:in `block (2 levels) in <class:UsersControllerTest>'
            test/functional/users_controller_test.rb:253:in `block in <class:UsersControllerTest>'

Hopefully this helps!

ViditChitkara · 2018-06-25T20:01:42Z

Hi @icarito, Sorry for the delay, was reinstalling plots on my system. This is related to tests I guess, hope redis will work now!! I'll push a patch to fix these tests.

ViditChitkara · 2018-06-25T20:48:01Z

The tests are passing now and we haven't got any redis related errors. Let's see if redis would work on production now. @jywarren , @icarito shall we merge this?

namangupta01 · 2018-06-25T21:02:32Z

@ViditChitkara How we can check this dashboard?

ViditChitkara · 2018-06-30T08:38:28Z

The changes seem good to me. Just wanted to ask if the contents of docker-compose-testing need to be copied to docker-compose-production??
Also, I would be needing sidekiq dashboard access to keep a check on errors. Can we keep it unauthenticated for sometime?? It is not a security issue as it gives stats of processed and erroneous requests.
@icarito, @jywarren?
Once this gets merged I'll work on digest email template improvement and any sidekiq follow-ups, if required.

jywarren · 2018-06-30T13:11:08Z

Yes, that's right to production and stable as well... and are you sure the requests themselves don't include visible personal information like even a username? Thanks!

…

On Sat, Jun 30, 2018, 4:39 AM Vidit ***@***.***> wrote: The changes seem good to me. Just wanted to ask if the contents of docker-compose-testing need to be copied to docker-compose-production?? Also, I would be needing sidekiq dashboard access to keep a check on errors. Can we keep it unauthenticated for sometime?? It is not a security issue as it gives stats of processed and erroneous requests. @icarito <https://github.com/icarito>, @jywarren <https://github.com/jywarren>? Once this gets merged I'll work on digest email template improvement and any sidekiq follow-ups, if required. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2772 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ-swEsWaZWBGV18Pykp160KWoaU5ks5uBzkEgaJpZM4UbcGm> .

ViditChitkara · 2018-06-30T14:17:09Z

@jywarren I don't see the db container in production docker compose file. However it is there in all other containers. Any particular reason for that?
And regarding sidekiq dashboard, it does not show any kind of usernames (usernames are not present in requests as well). However, I have found a way to password protect it. Using basic auth (https://github.com/mperham/sidekiq/wiki/Monitoring#standalone-with-basic-auth), but we need to put username and password as our server's environment variables.

ViditChitkara · 2018-06-30T14:25:09Z

Shall I do the auth thing in next pr??

icarito · 2018-06-30T14:36:35Z

Yes @vidit we're trying to standardize our docker-compose-*.yml files. Production currently uses an independent mysql container that is in a different compose file. As opposed to staging, we're running production in "host" network mode so we'll have to omit the `links` section and add `network_mode:host` to the sidekiq container configuration. For the case of redis container, you'll need to add a "ports" directive like this: ``` ports: - "127.0.0.1:3307:3306" ``` (With the correct port for Redis). This will make sure Redis port is only open for localhost. I'm aware this docker-compose setup is complex and we're looking to simplify it, thanks for your input and efforts!

…

On 30/06/18 09:17, Vidit wrote: @jywarren <https://github.com/jywarren> I don't see the db container in production docker compose file. However it is there in all other containers. Any particular reason for that?

ViditChitkara · 2018-06-30T14:51:19Z

Got it @icarito , I have made the final changes. If you want to have a look??

ViditChitkara · 2018-06-30T15:34:14Z

Okay, the tests are passing now!! @icarito , @jywarren I guess this is ready!

jywarren

If this looks good to @icarito it looks good to me!

ViditChitkara · 2018-06-30T16:11:05Z

cool! 👍

jywarren · 2018-07-03T18:17:43Z

Ack, sorry, this drifted out of sync as we merged a couple other things. @ViditChitkara can you resolve the conflicts and we can go ahead and merge this? Thanks!

jywarren · 2018-07-03T21:55:15Z

I went ahead and resolved the issue, i hope this passes!

jywarren · 2018-07-03T22:12:22Z

Should resolve #2779... but maybe it didn't? I see in stable:

+ make build
cp config/database.yml.example config/database.yml
cp db/schema.rb.example db/schema.rb
docker-compose down --remove-orphans
Circular dependency between web and db

Did I do something wrong in my resolving the conflicts here? It did pass Travis...

ViditChitkara · 2018-07-04T04:31:50Z

Oops I guess I missed a lot, sorry couldn't do much as I was out yesterday!! I'll have a look at circular dependency.

icarito · 2018-07-04T04:43:51Z

I got it @ViditChitkara ! New PR in #2977

icarito · 2018-07-04T04:47:16Z

If this was working in unstable and wasn't anymore there was a difference between docker-compose-unstable and docker-compose-stable.

Currently the difference is only the port number so I'll make a PR for making this also configurable by environment variable.

ViditChitkara · 2018-07-04T05:22:38Z

cool

ViditChitkara · 2018-07-04T05:23:17Z

also, if you could have a look at production container. It looks good from my side though!

icarito · 2018-07-04T05:25:33Z

ok this should work in stable with #2977 - I'll work on production one now.

ViditChitkara · 2018-07-04T06:17:32Z

Cool! I thought of this but then I saw this line in testing container.

…

On Wed, Jul 4, 2018, 11:30 AM Sebastian Silva ***@***.***> wrote: @ViditChitkara <https://github.com/ViditChitkara> ViditChitkara#2 <ViditChitkara#2> pls this is the actual fix — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2772 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AVkX0fSolbj9irMhiCCTFWD2X-g5Rjnjks5uDFn7gaJpZM4UbcGm> .

jywarren · 2018-07-07T00:00:42Z

https://publiclab.org/sidekiq 🎉

* changes in docker-compose.yml for redis fixes * minor chnanges * minor tweak * add redis url to web container * minor chane for testing * minor chane for testing * merge with master * changes in tests * Fix link to redis container * Fix directory reference for sidekiq volume * Fix circular dependency. * Add link to redis container to testing compose file * remove sidekiq admin constraints for testing * sample cron job for testing on unstable * changes for testing * minor changes * mysql connection fix * mysql connection fix 2 * mysql connection fix 3 * mysql connection fix 4 * Try link to db. * Add rails environment * Update users_controller_test.rb * minor chane for testing * merge with master * Fix circular dependency. * Add link to redis container to testing compose file * made docker-compose changes for production * minor change

ViditChitkara added in-progress summer-of-code labels Jun 5, 2018

ghost assigned ViditChitkara Jun 5, 2018

ghost added the in progress label Jun 5, 2018

namangupta01 reviewed Jun 5, 2018

View reviewed changes

ViditChitkara force-pushed the sidekiq-config branch from 25cc013 to b7adc02 Compare June 6, 2018 18:35

ViditChitkara mentioned this pull request Jun 11, 2018

Community Check-in #3 Summer Of Code Week 4 #2764

Closed

ViditChitkara force-pushed the sidekiq-config branch from b7adc02 to 213ce34 Compare June 14, 2018 20:24

ViditChitkara mentioned this pull request Jun 14, 2018

User settings page #2828

Closed

5 tasks

ViditChitkara added review-me and removed in progress in-progress labels Jun 18, 2018

ghost assigned icarito Jun 25, 2018

ViditChitkara force-pushed the sidekiq-config branch from 1537f5d to 4b77d2f Compare June 25, 2018 20:15

icarito added 2 commits June 30, 2018 12:15

Fix circular dependency.

6f230c1

Add link to redis container to testing compose file

03aa3f1

made docker-compose changes for production

027bd44

minor change

55b7018

jywarren approved these changes Jun 30, 2018

View reviewed changes

jywarren requested a review from icarito June 30, 2018 16:13

Merge branch 'master' into sidekiq-config

b3c9db8

jywarren merged commit b3defd3 into publiclab:master Jul 3, 2018

ghost removed the review-me label Jul 3, 2018

ViditChitkara mentioned this pull request Jul 4, 2018

Error connecting to redis #2779

Closed

mount sidekiq dashboard #2772

mount sidekiq dashboard #2772

Conversation

ViditChitkara commented Jun 5, 2018

ViditChitkara commented Jun 5, 2018

jywarren commented Jun 5, 2018

ViditChitkara commented Jun 5, 2018

plotsbot commented Jun 5, 2018 • edited Loading

namangupta01 commented Jun 5, 2018 • edited Loading

namangupta01 Jun 5, 2018

Choose a reason for hiding this comment

ViditChitkara Jun 6, 2018

Choose a reason for hiding this comment

ViditChitkara commented Jun 6, 2018

jywarren commented Jun 23, 2018

jywarren commented Jun 23, 2018

ViditChitkara commented Jun 23, 2018

jywarren commented Jun 23, 2018 via email

icarito commented Jun 23, 2018 via email

icarito commented Jun 25, 2018

icarito commented Jun 25, 2018

ViditChitkara commented Jun 25, 2018

ViditChitkara commented Jun 25, 2018

namangupta01 commented Jun 25, 2018

ViditChitkara commented Jun 30, 2018

jywarren commented Jun 30, 2018 via email

ViditChitkara commented Jun 30, 2018

ViditChitkara commented Jun 30, 2018

icarito commented Jun 30, 2018 via email

ViditChitkara commented Jun 30, 2018

ViditChitkara commented Jun 30, 2018

jywarren left a comment

Choose a reason for hiding this comment

ViditChitkara commented Jun 30, 2018

jywarren commented Jul 3, 2018

jywarren commented Jul 3, 2018

jywarren commented Jul 3, 2018

ViditChitkara commented Jul 4, 2018

icarito commented Jul 4, 2018 • edited Loading

icarito commented Jul 4, 2018

ViditChitkara commented Jul 4, 2018

ViditChitkara commented Jul 4, 2018

icarito commented Jul 4, 2018

ViditChitkara commented Jul 4, 2018 via email

jywarren commented Jul 7, 2018

plotsbot commented Jun 5, 2018 •

edited

Loading

namangupta01 commented Jun 5, 2018 •

edited

Loading

icarito commented Jul 4, 2018 •

edited

Loading