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

Bug in Event default created_at time #23

Closed
armstrys opened this issue Jan 2, 2023 · 4 comments
Closed

Bug in Event default created_at time #23

armstrys opened this issue Jan 2, 2023 · 4 comments

Comments

@armstrys
Copy link
Contributor

armstrys commented Jan 2, 2023

The default value for created_at in the Event class seems to hold the time.time() value from the time of import.

Example of a failing test below:

from nostr.event import Event
from nostr.key import PrivateKey
import time

time.sleep(2)

event = Event(public_key=PrivateKey().public_key.hex(), content='this is a test')
assert event.created_at == int(time.time())
@jeffthibault
Copy link
Owner

Hi @armstrys, the Event class sets the created_at field to the current time by default.

If I correctly understand what you are trying to do, you can do something like this:

curr_time = time.time()
event = Event(public_key, "this is a test", created_at=curr_time)
assert event.created_at == curr_time

@armstrys
Copy link
Contributor Author

Hi @jeffthibault! What I am noticing on my machine is that the default value is being set as soon as the module is imported as opposed to when an Event object is instantiated. So every event I was creating was carrying the same time stamp despite being created at different times. The time.sleep(2) call is creating a delay between the import and the instantiation to try to test this.

Basically the default time isn’t being recalculated for each event properly. If I’m seeing this wrong then I agree it would make more sense to keep it defined as the arg default.

flothjl pushed a commit to flothjl/python-nostr that referenced this issue Jan 18, 2023
@flothjl
Copy link

flothjl commented Jan 18, 2023

@armstrys I agree with you. Didn't realize you had opened a PR so I did, but have since deleted it. I think the problem lies in that the default args are determined on import, not initialization.

@jeffthibault
Copy link
Owner

Fixed in #24

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

No branches or pull requests

3 participants