Skip to content

logs added. Can you find all of the funny easter eggs and Shtick?#19

Merged
BenjyJack merged 2 commits intomainfrom
logging
Jul 20, 2021
Merged

logs added. Can you find all of the funny easter eggs and Shtick?#19
BenjyJack merged 2 commits intomainfrom
logging

Conversation

@BenjyJack
Copy link
Copy Markdown
Owner

No description provided.

@BenjyJack BenjyJack requested review from TGfor3 and removed request for TGfor3 July 19, 2021 19:18
book.setStoreID(storeID);
book.setStore(store);
EntityModel<Book> entityModel = assembler.toModel(repository.save(book));
logger.info("Book has been added");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it would be nice to include information about the book, using variable substitution like here: http://www.slf4j.org/api/org/slf4j/Logger.html

Book newBook = new Book(element.getAsJsonObject());
entityList.add(assembler.toModel(newBook));
}
logger.info("Batch request successfully executed by " + leader.getLeader());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

one again, lets use variable substitution


this.leader.setLeader(getLeader());

logger.info("Server initialized B-)");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

B- ?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

B-) is a sunglasses wearing emoticon

json.addProperty("id", this.id);
json.addProperty("address", this.url + "/bookstores/" + this.id);
createPutConnection(hubUrl, json);
logger.info("Server connected to network");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

again, here it would be good to add some context - id, address


private Long getLeader() throws Exception {
HttpResponse<String> response = createGetConnection(hubUrl + "/leader", this.url, id);
logger.info("Leader found. Mission Accomplished");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nice :)

entModelList.add(getAndParseBookStore(this.map.get(storeId)));
}catch(Exception e){
continue;
}catch(Exception ignored){
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this would be a good place for a log message

return ResponseEntity.status(HttpStatus.NO_CONTENT).build();
}catch (BookStoreNotFoundException e){
if(this.map.containsKey(storeID)){
logger.info("Corporate sabotage is Assur, except in a Karpeif against Ben & Jerry's");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hahaha

@SpringBootApplication
public class DatastoreApplication {

private static final Logger LOGGER= LoggerFactory.getLogger(DatastoreApplication.class);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if we arent gonna use it, I dont think there is any purpose

@yspotts
Copy link
Copy Markdown
Collaborator

yspotts commented Jul 19, 2021

Haha! Love the schtick.

A few comments for improvements:

  • Use variable substitution (I see you did so in a couple of examples) - lets do it consistently
  • Provide more context to log statements (what was added, what was updated)
  • I think it would be especially useful to add log statements (at warn or error) where we catch an exception, and especially when we catch the exception and then continue. It can be really challenging to troubleshoot in those cases without log statements.

@BenjyJack BenjyJack merged commit b045a8e into main Jul 20, 2021
@BenjyJack BenjyJack deleted the logging branch July 20, 2021 14:28
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.

2 participants