Skip to content
This repository has been archived by the owner on Oct 17, 2020. It is now read-only.

Rename Worker in master package to WorkerClient #10

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eugeneYWang
Copy link
Collaborator

This time it should be right.

@eugeneYWang
Copy link
Collaborator Author

BTW, the edit on go.sum and go.mod is automatically improved by GoLand. so I think those changes should be okay. Let me know if they should not be there.

@magicoder10 magicoder10 changed the title refactor worker struct to workerclient struct in master package Rename Worker in master package to WorkerClient Aug 31, 2020
go.mod Show resolved Hide resolved
master/master.go Outdated
// process the current site there is idle worker
worker := <-m.idleWorkers
// process the current site there is idle workerClients
worker := <-m.idleWorkerClients
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
worker := <-m.idleWorkerClients
workerClient := <-m.idleWorkers

master/server.go Outdated
workerID := s.master.RegisterWorker(worker)
fmt.Printf("Worker registed: ID(%d) IP(%s) PORT(%d) SECRET(%s)\n", workerID, request.Ip, int(request.Port), request.Secret)
workerID := s.master.RegisterWorker(workerClient)
fmt.Printf("WorkerClient registed: ID(%d) IP(%s) PORT(%d) SECRET(%s)\n", workerID, request.Ip, int(request.Port), request.Secret)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fmt.Printf("WorkerClient registed: ID(%d) IP(%s) PORT(%d) SECRET(%s)\n", workerID, request.Ip, int(request.Port), request.Secret)
fmt.Printf("Worker registered: ID(%d) IP(%s) PORT(%d) SECRET(%s)\n", workerID, request.Ip, int(request.Port), request.Secret)

master/master.go Outdated
Comment on lines 10 to 11
workerClients []WorkerClient
idleWorkerClients chan WorkerClient
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
workerClients []WorkerClient
idleWorkerClients chan WorkerClient
workers []WorkerClient
idleWorkers chan WorkerClient

Thank you making the changes. I feel calling them workers is more natural in this code. You may disagree though.

master/master.go Outdated
@@ -46,11 +46,11 @@ func (m *Master) ExploreWebsite(ctx context.Context, siteURL string) {
}
case link := <-linkCh:
go func(link string) {
// process the current site there is idle worker
worker := <-m.idleWorkers
// process the current site there is idle workerClients
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// process the current site there is idle workerClients
// process the current site there is idle worker

master/master.go Outdated
err := worker.FetchLinks(ctx, link)
if err != nil {
m.idleWorkers <- worker
m.idleWorkerClients <- worker
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
m.idleWorkerClients <- worker
m.idleWorkers <- worker

master/master.go Outdated
@@ -62,21 +62,21 @@ func (m *Master) ExploreWebsite(ctx context.Context, siteURL string) {
func (m *Master) FinishExtractingLinks(workerID int, links []string) {
m.linksCh <- links
go func() {
m.idleWorkers <- m.workers[workerID]
m.idleWorkerClients <- m.workerClients[workerID]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
m.idleWorkerClients <- m.workerClients[workerID]
m.idleWorkers <- m.workers[workerID]

master/master.go Outdated
func (m *Master) RegisterWorker(worker Worker) int {
workID := len(m.workers)
m.workers = append(m.workers, worker)
func (m *Master) RegisterWorker(workerClient WorkerClient) int {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func (m *Master) RegisterWorker(workerClient WorkerClient) int {
func (m *Master) RegisterWorker(worker WorkerClient) int {

@magicoder10
Copy link
Member

@eugeneYWang Sounds good!

@eugeneYWang
Copy link
Collaborator Author

suggested changes were applied. Does it look good?

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

Successfully merging this pull request may close these issues.

None yet

2 participants