Rasa: Status endpoint doesn't return the name of the actual model

[msamogh]

The /status currently returns the name of the deserialised model stored in the tmp folder rather than the name of the actual original model

• [x] implementation done
• [ ] adding tests

[btotharye]

I'll look at this soon, should be around https://github.com/RasaHQ/rasa/blob/master/rasa/server.py#L395 just making notes for myself

[btotharye]

@msamogh any suggestions on what we might should call this other status object since this model_file is used quite a bit in the code as the actual file for training and such, so I think we should still show that filename but also give something like original_model or something maybe to give the original filename.

Thoughts?

I'm still digging through to figure out the best place to add this because it doesn't appear the app.agent object from the server.py has anything but the model_directory value atm and none of the other objects I can see have this original value as a value anywhere yet.

[btotharye]

actually looks like I can get it from model.get_latest_model so maybe we should just call it "latest_model"?

Here is an example I made:

{"latest_model":"models\/20191017-072852.tar.gz","model_file":"\/var\/folders\/x5\/pkbj85f136zg0kr17ghzlhl40000gn\/T\/tmp2_fzo9r3","fingerprint":{"config":"99914b932bd37a50b983c5e7c90ae93b","core-config":"d89bec7f22c3af6c41bfd8fd35b5d56e","nlu-config":"482188dbce37868ed4f2cc45b57ef2d6","domain":466683804222406711,"messages":666668429431049818,"stories":625193187759202793,"trained_at":1571290120.951813221,"version":"1.3.9"},"num_active_training_jobs":0}

[btotharye]

https://github.com/RasaHQ/rasa/tree/fix_model_status is the code for this so far, mainly just need feedback on the name of latest_model

[msamogh]

I'm actually wondering if there's any reason to output the temporary path at all. We should just be able to replace it with the output of model.get_latest_model(). Of course, in that case, we'd have to wait for the next minor release (as it'd be a breaking change).

[btotharye]

yea the only reason I can think to keep it around would be for troubleshooting purposes in case we wanted a easy way to know where the actual tmp model file is, do you think that would be useful at all?

If not I can just update this and do a PR for it to replace model_file

[msamogh]

I mean you can always look at the identical file in the models/ directory to perform any inspection.

@wochinge Any thoughts on this?

[wochinge]

I think there is no reason to keep the tmp one around 👍 If the unpacking fails, there should be enough other errors.

[btotharye]

Still leaving this open a little longer since I need to write tests for this, so general FYI

[wochinge]

if you don't have time to tackle it this week, I'd actually create a separate issue for that.

[btotharye]

Actually I don't need to write tests they are already there since I only updated the name https://github.com/RasaHQ/rasa/blob/master/tests/test_server.py#L110 right? @wochinge

[wochinge]

True, but shouldn't we test that this actually a valid file + not in tmp?

[btotharye]

True, but shouldn't we test that this actually a valid file + not in tmp?

Yea good point, I'll add it

[btotharye]

created new issue https://github.com/RasaHQ/rasa/issues/4681 to track the tests @wochinge so can we close this one out now?