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

Fix bugs from the switch to the publicsuffixlist package #4

Merged
merged 2 commits into from
Feb 6, 2023

Conversation

mcdonnnj
Copy link
Member

@mcdonnnj mcdonnnj commented Feb 5, 2023

🗣 Description

Thie pull request fixes two issues with using the new Public Suffix List package we switched to in #2.

💭 Motivation and context

These two bugs (wrong return value and missing cache directory) cause breaking behavior.

🧪 Testing

I tested this change locally and in an updated cisagov/scanner Docker image.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.

This was missed when these functions were reworked. These functions no
longer return a tuple so these error returns should not return a tuple.
Before we can write to the cache directory in the `load_suffix_list()`
functions we need to ensure that the cache directory exists.
@mcdonnnj mcdonnnj added the bug This issue or pull request addresses broken functionality label Feb 5, 2023
@jsf9k jsf9k marked this pull request as ready for review February 5, 2023 17:11
@jsf9k jsf9k requested a review from dav3r February 5, 2023 17:11
@jsf9k jsf9k merged commit def3c28 into master Feb 6, 2023
@jsf9k jsf9k deleted the bugfix/fix_issues_with_psl_updating branch February 6, 2023 15:23
jsf9k added a commit to cisagov/orchestrator that referenced this pull request Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue or pull request addresses broken functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants