Conversation
Since i have not run this as a dag yet this is the first attempt at this.
create-tokens.py
Outdated
| AND approved = true | ||
| AND token_id IS NULL | ||
| LIMIT 3000 | ||
| LIMIT .format(mintLimit) |
There was a problem hiding this comment.
not sure about this one though... syntax might be wrong
There was a problem hiding this comment.
We need to test it fully, I remember this syntax for variables:
f""" some {variable}"""
@CareyAltgilbers can you also help on this?
There was a problem hiding this comment.
@dadiorchen Yes, I'll be happy to take a look at it.
There was a problem hiding this comment.
Hello,
It looks like on line 61 kwards is a typo and should be kwargs.
In the SQL statement on line 116 we should be able to change LIMIT .format(mintLimit) to Limit {} and then on line 117 change that to """.format(entityId, mintLimit))
That gives us two unnamed variables in this SQL statement, one on line 110 in a function call and another in line 116.
I ran the code in a test script and did not get any syntax errors, if you have some test data I can use for the entityId and mintLimit I would run the test script again.
cursor.execute("""
select id, uuid, token_id from trees
where
planter_id IN (
select id from planter
where
organization_id IN (
select entity_id from getEntityRelationshipChildren({})
)
)
AND active = true
AND approved = true
AND token_id IS NULL
LIMIT {}
""".format(entityId, mintLimit))
There was a problem hiding this comment.
changed it. Should be fine now. see new change request
| print('dryRun is None') | ||
| return | ||
|
|
||
| if mintLimit is None: |
There was a problem hiding this comment.
I don't know the busines logic behind this but I will recommend setting a default minLimit rather than exiting the job when minLimit is None.
There was a problem hiding this comment.
business logic works better with a limit every time. that is fine
sebastiangaertner
left a comment
There was a problem hiding this comment.
@CareyAltgilbers this should be ok now or?
CareyAltgilbers
left a comment
There was a problem hiding this comment.
Those updates look good.
Since i have not run this as a dag yet this is the first attempt at this.