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

move weekly digest email to sidekiq-cron and remove whenever #401

Merged
merged 7 commits into from
Feb 25, 2020

Conversation

markets
Copy link
Collaborator

@markets markets commented Aug 15, 2018

Closes #373
Closes coopdevs/timeoverflow-provisioning#24
Closes #544
Closes #548

EXTRA:

  • update more dev deps: rubocop, byebug, fabrication, capybara, webdrivers
  • fixing CI (feature specs)

INFRA CHANGES & TESTING
👀 #401 (comment)

Copy link
Contributor

@enricostano enricostano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍

@enricostano
Copy link
Contributor

To test this we first need to solve coopdevs/timeoverflow-provisioning#36

@markets
Copy link
Collaborator Author

markets commented Aug 21, 2018

Ok @enricostano thanks! I think this will also close #369

@enricostano
Copy link
Contributor

Cool @markets , many thanks for improving this PR! 💪

Just two questions:

  1. What would be the best strategy to test it?
  2. I guess we should plan some kind of pre-post deploy operations for production to remove the existent cron job, right?

@markets
Copy link
Collaborator Author

markets commented Mar 11, 2019

Hi @enricostano

  1. the code inside the Job class is just a copy/pasta from the old Service class (with find_each instead of each), so in terms of logic, it should work exactly as it works now. To ensure that, I think we can use the Sidekiq UI, as cron jobs are displayed there and they have an "Enqueue Now" button:

Captura de pantalla 2019-03-11 a la(s) 21 21 32

  1. We should remove the crontab line generated by the whenever gem, this can be done by hand (crontab -e) or using an option provided by the gem: whenever -c or --clear-crontab. We should ✂️ remove things in the provisioning system too: https://github.com/coopdevs/timeoverflow-provisioning/search?q=cron&type=Code.

Also, you commented in the past, we'll need to solve this first coopdevs/timeoverflow-provisioning#36.

Let me know what do you think and what can I do to help moving this forward!

@sseerrggii
Copy link
Contributor

🤔 I think I will need som help to test that @enricostano also I think we have som troubles with push notifications, so one day we can sit together and test thinks looking at Sidekiq 🙏

@enricostano
Copy link
Contributor

Blocked by coopdevs/timeoverflow-provisioning#36

@markets markets removed the blocked label Dec 10, 2019
@markets
Copy link
Collaborator Author

markets commented Jan 21, 2020

Just updated with current develop! I also "unblocked" this PR, as the main blocker coopdevs/timeoverflow-provisioning#36 was already solved.

@sseerrggii
Copy link
Contributor

Testing it on staging i get this error

FireShot Capture 377 -  STAGING  Sidekiq - staging timeoverflow org

…::AssociationRelation + update to latest Rubocop
@markets markets force-pushed the remove_whenever branch 3 times, most recently from fcfc185 to 121bf3f Compare February 23, 2020 16:43
@markets markets mentioned this pull request Feb 23, 2020
@markets
Copy link
Collaborator Author

markets commented Feb 23, 2020

Yay! Travis is passing ✅ now! It's already failing in master, and not related to this branch, but I fixed it here in order to have green build for this branch.

@sseerrggii I also fixed the error you got last day. Thanks!

@sauloperez
Copy link
Collaborator

Thanks a ton @markets ! what a mess with the locale 🙈 @mperezv this is relevant to you ☝️

@sseerrggii
Copy link
Contributor

Now Works fine, thanks @markets 💚

@sauloperez sauloperez merged commit c47fe2b into develop Feb 25, 2020
@sauloperez sauloperez deleted the remove_whenever branch February 25, 2020 15:55
sauloperez added a commit to coopdevs/timeoverflow-provisioning that referenced this pull request Feb 28, 2020
Since coopdevs/timeoverflow#401 it's managed by
sidekiq-cron, like we already do with push notifications.
@sauloperez
Copy link
Collaborator

Finally up and working in production! thanks a lot @markets 🎉 ! TO is now cron-free and relies on Sidekiq for all kinds of async stuff 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants