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

[Debug] Without the app.log file, the app.py and the docker will exit #4

Closed
wants to merge 4 commits into from

Conversation

xiaoranzhou
Copy link

[Debug] Without the app.log file, the app.py and the docker will exit

It will show exit(3) without the folder and the app.log in it
@slobentanzer
Copy link
Contributor

@fengsh27 could you please check if this is fine to merge?

@fengsh27
Copy link
Collaborator

Looks good

@fengsh27
Copy link
Collaborator

@xiaoranzhou great work! I suggest adding a file called logs/placeholder instead of logs/app.log. This change will help prevent accidentally committing our own app.log to the repo. Additionally, please update .gitignore file to include logs/*.log.

@slobentanzer
Copy link
Contributor

@fengsh27 @xiaoranzhou is this just about keeping the folder, i.e., the actual file is not important? If that is the case, I usually add a .gitkeep file, which has the same function as a placeholder would have, but is more self-explanatory as well as hidden.

@fengsh27
Copy link
Collaborator

Yes, I believe it exited due to failing to write app.log to folder ./logs. In this case, .gitkeep is a better option.

@xiaoranzhou
Copy link
Author

As this is under the scope of development, I will close this issue. 😄

@slobentanzer
Copy link
Contributor

@xiaoranzhou you also closed the PR, was that your intent?

@xiaoranzhou
Copy link
Author

Yes, this will be solved by using empty folder and .gitkeep right 😄 ? So I closed it.

@slobentanzer
Copy link
Contributor

You also changed some logic in the conversation manager in this PR

@slobentanzer
Copy link
Contributor

Also, why not implement the .gitkeep solution in this PR and then merge?

@xiaoranzhou
Copy link
Author

I added this quick and dirty solution to make sure biochatter-server run. 😄
The wasm was relying the biochatter-server. If biochatter-server does not run, you will not be able to test the wasm part.
Now I have changed the wasm part to make it independent of the biochatter-server.
So I will only get back to the biochatter-server, when you finshed the debug.

There are currently also other issues with the port setting the respons format.
Maybe finish "port" and "respons" first.

@slobentanzer
Copy link
Contributor

Agree re the port, we should address #2. What is the response issue? I am not sure I know that one. Can we create an issue?

@xiaoranzhou
Copy link
Author

#6 created, I will add more debug information after the wasm merge finished 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants