model id specified in dockerfile

[ErikKaum]

What is this?

A change to specify the model id in only one place => the dockerfile.

Why?

To avoid mistakes of changing the model id in one of the current places and therefore loading several models

How did you test to ensure no regressions?

Built this version of the serverless template on banana and tested inference => everything worked as expected.

If this is a new feature what is one way you can make this break?

[erik-dunteman]

Thanks for this. I agree that it's good practice to have the model in a single place, at least in the case of this example Huggingface models.

Problem: the purpose of burt-base-uncased is for the BERT example only, and by putting it as an env var rather than verbose in app.py and download.py, it can lead to users thinking that the env var is a required pattern.

I fear that this pattern will lead to users having the following confusion:

• is "MODEL_ID" an ID from the Banana UI? What does that mean? (it's not a banana thing, it's a huggingface thing)
• can I just put any huggingface model in MODEL_ID and it'd work? (no, different HF models have different pre/post-processing code in app.py)
• does Banana need me to have MODEL_ID for this to deploy? (no, MODEL_ID is just part of the example HF bert model, and not a systematic thing)
• why does this only work in docker? (because this PR doesn't instruct users to set the env var in a local env)

Changes I'd need to see for this PR:

1. Add documentation in the README and comments in app.py + download.py that explain that we're referencing an env var to avoid duplicating strings, how to set that env var, and how that env var is only for the purpose of the example BERT model.
2. MODEL_ID renamed so that it's clearly not a required part of the Banana system.

Alternatively, I'll ask the question: what was the root cause of this problem in the first place? Users forgot to update the huggingface model ID across app.py and download.py, so rather than the added complexity and burden of documentation of this MODEL_ID change, is there anything different we can do with the UX to make it so app.py and download.py are more understandable?

[ErikKaum]

Thanks for the comments!

This is a really good point that you're bringing up. I'd go with the changed variable name & clear documentation. We'll avoid "silent" bugs and with a clear instruction of how to test locally we'll avoid people confusing this with something associated with banana.

I'll also add some notes in the README on setting the env variable locally 👍

[erik-dunteman]

Looks good to me!