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

update timeouts to give examples in Golang durations #46

Closed
wants to merge 3 commits into from

Conversation

willaaam
Copy link

@willaaam willaaam commented Feb 1, 2019

Description

Motivation and Context

  • I have raised an issue to propose this change (required)

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@derek
Copy link

derek bot commented Feb 1, 2019

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off.
That's something we need before your Pull Request can be merged. Please see our contributing guide.

@alexellis
Copy link
Member

Derek set title: update timeouts to give examples in Golang durations

@derek derek bot changed the title Update README.md - took me hours of messing to find out that env variable documentation was just wrong. Please check other env variables if the same holds true for them. update timeouts to give examples in Golang durations Feb 1, 2019
@alexellis
Copy link
Member

All timeout values need to be specified in Golang durations. Please feel free to use community support if you get stuck in the future :-)

https://docs.openfaas.com/community

No question too small to be asked there.

Out of curiosity what were you attempting to do?

Alex

@willaaam
Copy link
Author

willaaam commented Feb 1, 2019

Hi Alex,

Thanks! And yeah, I should've, hahaha. We're using openfaas-of in front of tensorflow serving, so that's also why I needed to extend the default timeout. It's not a super-long running process, but model inference takes some time.

The reason for using openfaas-of is that loading a model using the fork method takes a lot of time. For some other bits of our AI pipeline it makes a lot more sense to fork a process and do something small - it mainly prevents a lot of overhead in our code and keeps the code simple and easy to replace with something newer or better. Such as a tokenizer for example.

Does that make sense?

Kind regards,

Willem

@alexellis
Copy link
Member

Please can you run git commit -s --amend and repush?

The usecase sounds really interesting. Speak on slack?

Alex

@willaaam
Copy link
Author

willaaam commented Feb 1, 2019

I kind of did everything via the github website as it was a quicky - apologies.

Done.

willaaam and others added 2 commits February 1, 2019 15:53
Signed-off-by: Willem Mobach <willem.mobach@outlook.com>
@alexellis
Copy link
Member

Oh dear that didn't go very well. We can't accept the merge commit. Just use git reset and push it up again or let me know if you want me to do the git piece.

@alexellis
Copy link
Member

Thanks for your patience!

@alexellis alexellis closed this in 5be7ca3 Feb 1, 2019
@openfaas openfaas locked and limited conversation to collaborators Feb 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants