fossasia / susi_api_wrapper

SUSI AI API Wrapper http://susi.ai
Apache License 2.0
1.48k stars 31 forks source link

Added Server checkers #75

Open sansyrox opened 5 years ago

sansyrox commented 5 years ago

Base PR of issue : https://github.com/fossasia/susi_linux/issues/464 Added Server Checker functions that check if the local server is active or not

hongquan commented 5 years ago

I think the behaviour of "having a background task to check and switch between local server and remote server" is just a specific need of an application that uses this lib.

When you are making a lib, you have to make it agnostic to application. If this lib can only be used for one application, better baking it into application.

There are other issues:

I refuse to merge this PR.

sansyrox commented 5 years ago

@hongquan , I don't understand. What do you mean by "lib" ?

hongquan commented 5 years ago

"lib" = "library", I mean this "susi_api_wrapper". You moved it out of "susi_linux" to act as a library.

sansyrox commented 5 years ago

Must merge for this PR to work .

sansyrox commented 5 years ago

@hongquan @Orbiter please have a look again.

hongquan commented 5 years ago

I will leave @Orbiter to decide to merge this or not. My opinion is to refuse this, because I don't agree to put a background task into a library. It should be done in application (susi_linux), not library (susi_api_wrapper).