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

Avoid crashing when writing of config file fails #91

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kislyuk
Copy link
Member

@kislyuk kislyuk commented Jan 22, 2018

No description provided.

@kislyuk kislyuk requested review from ttung and Bento007 January 22, 2018 05:29
@ghost ghost assigned kislyuk Jan 22, 2018
@ghost ghost added the code review label Jan 22, 2018
if not (e.errno == errno.EEXIST and os.path.isdir(self.config.user_config_dir)):
raise
if os.path.exists(swagger_filename):
with open(swagger_filename) as fh:
Copy link
Member

Choose a reason for hiding this comment

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

technically, you should wrap this in a try-catch block if your goal is to make it not crash. file existence is not sufficient.

I would just do:

try:
    with open(swagger_filename) as fh:
        self.__class__._swagger_spec = json.load(fh)
except Exception:
    try:
        Path(self.config.user_config_dir).mkdir(parents=True, exist_ok=True)
        with open(swagger_filename, "wb") as fh:
            fh.write(res.content)
    except IOError as e:
        logger.error(e)
    self.__class__._swagger_spec = res.json()

@kbergin
Copy link

kbergin commented Apr 12, 2018

Bumping on open PRs that haven't been updated in a while. Anything to do here?

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

Successfully merging this pull request may close these issues.

3 participants