|
great, I have been working on the similar PR, but you did it first and way better 💯 |
Sorry, something went wrong.
|
@james-crabtree-sp Thanks again for putting this PR together. I think in order to merge, you'll need to sign your commits: https://docs.feast.dev/project/development-guide#signing-off-commits |
Sorry, something went wrong.
|
@james-crabtree-sp Thanks again for putting this PR together. I think in order to merge, you'll need to sign your commits: https://docs.feast.dev/project/development-guide#signing-off-commits I think they are all signed off already. I see that the DCO task that checks this completed successfully https://github.com/feast-dev/feast/pull/3789/checks?check_run_id=17541354092 |
Sorry, something went wrong.
|
@james-crabtree-sp Thanks again for putting this PR together. I think in order to merge, you'll need to sign your commits: https://docs.feast.dev/project/development-guide#signing-off-commits I think they are all signed off already. I see that the DCO task that checks this completed successfully https://github.com/feast-dev/feast/pull/3789/checks?check_run_id=17541354092 Sorry for the confusion, I was referring to the requirements to merge, and linked to the wrong documentation. This is the requirement that I was referring to: Merging is blocked |
Sorry, something went wrong.
|
hey @james-crabtree-sp this is a great PR, you need any hands to make it merged? I see there's a lot of commits in the past unverified if you don't mind I can create another PR to mimic this one |
Sorry, something went wrong.
|
@whoahbot @sudohainguyen Ahh, I understand now. Commits are now verified |
Sorry, something went wrong.
|
@whoahbot @sudohainguyen Ahh, I understand now. Commits are now verified exactly 🤣 looks good now but you need to run code format though |
Sorry, something went wrong.
|
@whoahbot @sudohainguyen Ahh, I understand now. Commits are now verified exactly 🤣 looks good now but you need to run code format though Noticed that too. Weird rebase issue. Should be fixed now |
Sorry, something went wrong.
|
@james-crabtree-sp I've been adopting your changes on my side so far do we need to wait the job to be done, like forever without timeout? |
Sorry, something went wrong.
|
@james-crabtree-sp I've been adopting your changes on my side so far do we need to wait the job to be done, like forever without timeout? Well, this PR adds the configurable active_deadline_seconds setting, which can be used to impose a timeout on the kubernetes jobs. Does that answer your question? |
Sorry, something went wrong.
|
@james-crabtree-sp I've been adopting your changes on my side Well, this PR adds the configurable active_deadline_seconds setting, which can be used to impose a timeout on the kubernetes jobs. Does that answer your question? cool clear |
Sorry, something went wrong.
What this PR does / why we need it:
This PR fixes 2 issues encountered while attempting to run very large materializations (Several billion rows as part of an initial run of online materialization).
Which issue(s) this PR fixes:
Fixes #3786
Fixes #3787
Fixes #3788