Skip to content

Add jq filter#626

Merged
thp merged 9 commits into
thp:masterfrom
robgmills:add-jq-filter
Apr 9, 2021
Merged

Add jq filter#626
thp merged 9 commits into
thp:masterfrom
robgmills:add-jq-filter

Conversation

@robgmills
Copy link
Copy Markdown
Contributor

Adds the ability to select content from JSON data using jq - a lightweight JSON processor - filter syntax. The jq filters are similar to CSS or XPath selectors, with added operators to transform the data.

Addresses #592 in a different way.

@robgmills
Copy link
Copy Markdown
Contributor Author

Thanks for sharing this project with us! I'm not super well-versed in Python projects and this has been a good experience.

I can see in the Travis job how to run tests and that I have some syntax issues in the documentation. I'll add a test or tests for this filter - is there anything in particular you want me to test for? I'll also clean up the documentation syntax issues I introduced.

@robgmills
Copy link
Copy Markdown
Contributor Author

These documentation driven tests are pretty cool! I'll have to use them as inspiration later.

Sorry for the disjointed commits - I normally try to keep things cleaner. This was a good learning experience! I'm excited to hear your thoughts!

Comment thread lib/urlwatch/filters.py Outdated
Comment on lines +878 to +888
json.loads(data)
except ValueError:
raise ValueError('The url response contained invalid JSON')

if 'query' not in subfilter:
raise ValueError('{} filter needs a query'.format(self.__kind__))

if jq is None:
raise ImportError('Please install jq')

return jq.text(subfilter['query'], json.loads(data))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

One "optimization" here could be to store the result of json.loads(data) in line 878, and re-use the result in line 888 to not have to re-parse the JSON data.

Copy link
Copy Markdown
Owner

@thp thp left a comment

Choose a reason for hiding this comment

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

Nice and clean. One comment about avoiding double-parsing of JSON. Also, you could add this change to CHANGELOG.md.

@robgmills
Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @thp!

@thp thp merged commit 9186328 into thp:master Apr 9, 2021
@thp
Copy link
Copy Markdown
Owner

thp commented Apr 9, 2021

Merged, thanks!

@smithl
Copy link
Copy Markdown

smithl commented Apr 19, 2021

Appreciate this addition very much, nice work!

@robgmills robgmills deleted the add-jq-filter branch April 20, 2021 13:25
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