Skip to content

DM-36449: Add check to test is var is there before deleting#22

Merged
mfisherlevine merged 1 commit intomainfrom
tickets/DM-36449
Oct 4, 2022
Merged

DM-36449: Add check to test is var is there before deleting#22
mfisherlevine merged 1 commit intomainfrom
tickets/DM-36449

Conversation

@mfisherlevine
Copy link
Copy Markdown
Contributor

No description provided.

# using an instrument label raises a FileNotFoundError
with unittest.mock.patch.dict('os.environ'):
del os.environ['DAF_BUTLER_REPOSITORY_INDEX']
if 'DAF_BUTLER_REPOSITORY_INDEX' in os.environ: # can't del unless it's already there
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.

It's more compact to say os.environ.pop('DAF_BUTLER_REPOSITORY_INDEX', None), but it might be less clear.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think I like the explicit way in this instance.

Copy link
Copy Markdown
Collaborator

@ktlim ktlim left a comment

Choose a reason for hiding this comment

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

This change alone is fine. I haven't looked at any of the other code, though, and problems remain with packages in lsst_sitcom (e.g. missing summit_extras).

@mfisherlevine
Copy link
Copy Markdown
Contributor Author

This change alone is fine. I haven't looked at any of the other code, though, and problems remain with packages in lsst_sitcom (e.g. missing summit_extras).

Yup, woke up to that fail on Jenkins. Should just be the fact that summit_extras wasn't in the table file for rubintv_production so I've added that to this same ticket number and am rerunning now...

@mfisherlevine mfisherlevine merged commit 5bc6417 into main Oct 4, 2022
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