ibnesayeed / linkextractor

A Docker tutorial using a link extraction application example
https://training.play-with-docker.com/microservice-orchestration/
MIT License
51 stars 54 forks source link

Without flush log file is empty. #3

Closed alfredleo closed 5 years ago

ibnesayeed commented 5 years ago

Thanks @alfredleo for the PR, but I feel I/O should only be flushed explicitly when it is mission critical. In general it is not a good practice to add flush after every write operation as it slows down the process and negates all the I/O optimizations done by programming languages and the OS itself via buffering I/O writes. When there are enough data (more than the buffer size) for the log, it will automatically be written out. Also, when the process is killed and file is closed, any residue buffer is written out. This is why, in the illustration of the code, we first kill the container and the show that the log persisted. Besides, this additional line is not adding anything to help the reader understand the application any better.

Do you still feel we should merge it? I am open for opinions.

alfredleo commented 5 years ago

I just up for a working example. In that case using logger would be better. Ex:

require "logger"
cache_log = Logger.new("logs/extraction.log")
cache_log.debug "#{Time.now.to_i}\t#{cache_status}\t#{url}"

As we generally test the log file via sending request. And here the log file fails to show up instantly on each request. If you approve I will rework request.

ibnesayeed commented 5 years ago

I am assuming, you came here from https://training.play-with-docker.com/microservice-orchestration/, if so, you would realize that the purpose of this repo is not to teach people Ruby or Python or any other programming language for that matter, instead, to illustrate how micro-services are organized in Docker containers. That's why I kept the code to minimum. Adding logger will increase number of lines in the code for users to process as they go through the illustration. Also, logger by default is very verbose, which will force us to update the tutorial accordingly or we must customize the logger instance heavily, hence adding more code.

Besides, logger will perhaps only ensure that transactions are logged immediately (unless customized differently), which can very well be achieved by your current PR as it is, but the question is, is it really critical for the purpose of this tutorial? If so, I can go ahead and merge it. Though, I would love to know, was it frustrating or yielded any unexpected surprises as you were following the tutorial or you want it just to be more explicit?

alfredleo commented 5 years ago

In that case leave it as it is. Maybe adding a hint that log is displayed after ending docker would help. Yes, I came from microservice-orchestration. Actually, I plan to develop this project furthermore. But that is out of scope of this training. This training course is awesome.

ibnesayeed commented 5 years ago

I appreciate your kind words and feedback @alfredleo. Please feel free to extend it by adding more steps here in this repo or in a forked one. Those steps can then either be utilized in the same tutorial on the training site by updating the lesson or adding a separate lesson, depending on the scope of what you bring to the table.

One logical next step I was thinking about adding was to finally customize the compose file and deploy it using native Swarm mode as a Stack. However, there is already a chapter that talks about it, so I thought it perhaps is OK to leave it there as an assignment.

ibnesayeed commented 5 years ago

Closing it now. @alfredleo please feel free to keep me in the loop if you extend it.

jxc876 commented 2 years ago

I was following step 6 and was confused why the log did not show anything (while docker-compose was still running). After reading these comments I realized I need to bring down docker compose so the file can be flushed.