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

Added a decorator #148

Merged
merged 10 commits into from
Feb 2, 2021
Merged

Added a decorator #148

merged 10 commits into from
Feb 2, 2021

Conversation

RHagenaars
Copy link
Contributor

I've added a decorator for conveniently defining jobs from functions.

@Naramsim
Copy link

Can you write pls how to use this decorator?

Thanks

@dbader
Copy link
Owner

dbader commented Jul 25, 2017

Thanks RHagenaars for creating this PR. To tell you the truth I'm on the fence whether this should be a part of schedule itself or an FAQ item.

A couple of thoughts here:

  • Please use functools.wraps to ensure the metadata of the wrapped function is copied over.
  • The code example doesn't import repeat()
  • I don't think the code example will render correctly in the HTML docs, this would also need fixing.

@RHagenaars
Copy link
Contributor Author

@Naramsim the usage would be as follows:

@repeat(every(10).minutes)
def f():
  print('This function is repeated.')

@dbader thanks for taking time to consider it. About your thoughts:

Please use functools.wraps to ensure the metadata of the wrapped function is copied over.

That's not necessary in this case. If you look closely, you'll see that the actual decorated function is returned and not some wrapper.

The code example doesn't import repeat()

Good catch! I've added that import.

I don't think the code example will render correctly in the HTML docs, this would also need fixing.

I'm not sure how to fix that. Could you elaborate some more on what exactly needs to be fixed? I took the docstring on top of the file for an example.

I'm on the fence whether this should be a part of schedule itself or an FAQ item

That's up to you of course. I personally like decorators for things like this, because it allows you to put coupled statements - the function and the schedule - close to each other. I think that in most cases, the code is more clear when these are not separated.

antwal added a commit to antwal/schedule that referenced this pull request Nov 25, 2018
antwal added a commit to antwal/schedule that referenced this pull request Nov 25, 2018
@SijmenHuizenga SijmenHuizenga linked an issue Dec 3, 2020 that may be closed by this pull request

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@coveralls
Copy link

coveralls commented Jan 31, 2021

Coverage Status

Coverage remained the same at 100.0% when pulling 237593f on RHagenaars:master into cc994b3 on dbader:master.

@SijmenHuizenga
Copy link
Collaborator

Hi @RHagenaars, Thanks a lot for submitting this pr! I'm thrilled to add this to the library 💯

Just now I've added support for passing arguments to the function via the decorator. And I've added some examples to the docs. Would you like to let me know what you think of this?

@SijmenHuizenga SijmenHuizenga self-requested a review January 31, 2021 21:01
@SijmenHuizenga SijmenHuizenga added this to the 1.1.0 milestone Jan 31, 2021
Copy link
Contributor Author

@RHagenaars RHagenaars left a comment

Choose a reason for hiding this comment

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

Looks good to me, @SijmenHuizenga ! I really like the addition of passing arguments.

SijmenHuizenga and others added 2 commits February 2, 2021 21:34
Co-authored-by: Ramon Hagenaars <ramonhagenaars@gmail.com>
@SijmenHuizenga
Copy link
Collaborator

Thanks for the review @RHagenaars! Let's merge 🚀

@SijmenHuizenga SijmenHuizenga merged commit 170b63a into dbader:master Feb 2, 2021
MartinThoma pushed a commit to MartinThoma/schedule that referenced this pull request Feb 8, 2021
* Added 'repeat' decorator and written a test. Increased the version by 0.0.1.

* Removed the type of the given job for python 2.* compatibility.

* Added some newlines to comply with FLAKE8

* Added import for 'repeat' in the code example

* Added decorator docs and the ability to pass arguments to the job through the decorator

* Fix FLAKE8

* Added rhagenaars to authors

* Apply suggestions from code review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend schedule to work as a decorator
5 participants