Skip to content

Issue : Undefined behaviour if Event contructor is called with non-existant file#121

Open
mehulmj wants to merge 4 commits intomapaction:masterfrom
mehulmj:master
Open

Issue : Undefined behaviour if Event contructor is called with non-existant file#121
mehulmj wants to merge 4 commits intomapaction:masterfrom
mehulmj:master

Conversation

@mehulmj
Copy link
Copy Markdown

@mehulmj mehulmj commented Mar 14, 2021

No description provided.

@qlty-cloud-legacy
Copy link
Copy Markdown

Code Climate has analyzed commit 15bd131 and detected 0 issues on this pull request.

View more on Code Climate.

@andrewphilipsmith
Copy link
Copy Markdown
Contributor

Hi Mehul Jain,

Thank you for the pull request. There are a couple of changes we would require before we can merge this:

  • We enforce pep8 on this repo using the flake8 module. See here for an example of the errors - https://travis-ci.com/github/mapaction/mapactionpy_controller/jobs/490870279
  • Because of the above I've tested the code directly. However reading it, I think that if the event_file does not exist it would now create an Event object with no attributes - this is not the desired behaviour.

The desired behaviour is that:

  • The constructor can only return a valid Event object (with meaningful values for all of the attributes).
  • If the event_file does not exist, an Exception is raised. This exception must have an error message describing which file could not be read and how it was to be used. MapChef handles hundreds of files, so the error message must provide the relevant context.
  • The error message should also be passed to the logging module.

I hope that helps.

@andrewphilipsmith andrewphilipsmith self-requested a review April 7, 2021 21:35
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