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

Add fail_on_error and filter_invalid options to load command #73

Merged
merged 5 commits into from
Nov 17, 2015

Conversation

pmeulen
Copy link
Contributor

@pmeulen pmeulen commented Oct 12, 2015

As discussed in #62
Added some tests as well. Let me know what you think.

@@ -438,15 +466,20 @@ def load(req, *opts):
elif os.path.exists(url):
if os.path.isdir(url):
log.debug("directory %s verify %s as %s via %s" % (url, params['verify'], params['as'], params['via']))
req.md.load_dir(url, url=params['as'], validate=opts['validate'], post=post)
req.md.load_dir(url, url=params['as'], validate=opts['validate'], post=post, fail_on_error=opts['fail_on_error'], filter_invalid=opts['filter_invalid'])
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm starting to wonder if we should refactor the argument passing to use named arguments instead. Opinions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean in the call to load_dir()? Or more generally?

I agree that req.md.load_dir(directory=url, url=params['as'], validate=opts['validate'], post=post, fail_on_error=opts['fail_on_error'], filter_invalid=opts['filter_invalid']) is more readable.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant more generally - these arrays are passed to the worker threads...

@landscape-bot
Copy link

Code Health
Repository health increased by 0.31% when pulling cc823ef on pmeulen:add_load_error_options into 0f2ae83 on leifj:master.

@pmeulen pmeulen closed this Nov 16, 2015
@pmeulen pmeulen deleted the add_load_error_options branch November 16, 2015 16:22
@leifj
Copy link
Contributor

leifj commented Nov 16, 2015

did you find a better solution?
(I was about to merge this...)

@pmeulen pmeulen restored the add_load_error_options branch November 17, 2015 08:45
@pmeulen
Copy link
Contributor Author

pmeulen commented Nov 17, 2015

Thinking you already merged this I removed the branch from my fork, that apparently made github close this PR. I reopend it, if you could merge it, that would be great.

@pmeulen pmeulen reopened this Nov 17, 2015
leifj added a commit that referenced this pull request Nov 17, 2015
Add fail_on_error and filter_invalid options to load command
@leifj leifj merged commit 47b025c into IdentityPython:master Nov 17, 2015
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.

3 participants