Skip to content
This repository was archived by the owner on Aug 30, 2024. It is now read-only.

Conversation

@smithsz
Copy link
Contributor

@smithsz smithsz commented Sep 11, 2018

Checklist

  • Tick to sign-off your agreement to the Developer Certificate of Origin (DCO) 1.1
  • Added tests for code changes or test/build only changes
  • Updated the change log file (CHANGES.md|CHANGELOG.md) or test/build only changes
  • Completed the PR template below:

Description

Allow arbitrary query parameters to be passed to custom changes filters.

Fixes #402.

Approach

Add new ANY_ARG and ANY_TYPE to list of valid changes arguments:

_CHANGES_ARG_TYPES = {
    'conflicts': (bool,),
    'doc_ids': (list,),
    'filter': (STRTYPE,),
    'include_docs': (bool,),
    'style': (STRTYPE,),
    ANY_ARG: ANY_TYPE  # pass arbitrary query parameters to a custom filter
}

Schema & API Changes

No change.

Security and Privacy

No change.

Testing

Includes additional unit test.

Monitoring and Logging

No change.

@smithsz smithsz force-pushed the 402-pass-params-to-custom-changes-filter branch from d5da3c3 to 8dd861a Compare September 11, 2018 15:44
self.assertSetEqual(set([x['id'] for x in changes]), expected)
self.assertTrue(str(feed.last_seq).startswith('100'))

def test_invalid_argument(self):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is no longer valid as passing foo='bar' is now accepted under ANY_ARG: ANY_TYPE.

@ricellis ricellis added this to the 2.next milestone Sep 11, 2018
Copy link
Member

@ricellis ricellis left a comment

Choose a reason for hiding this comment

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

+1, but please add the missing copyright update

"""

import json

Copy link
Member

Choose a reason for hiding this comment

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

This needs a copyright update

@smithsz smithsz force-pushed the 402-pass-params-to-custom-changes-filter branch from 8dd861a to 03385dc Compare September 12, 2018 09:25
@emlaver
Copy link
Contributor

emlaver commented Sep 12, 2018

@smithsz There's a couple cases that cause a CloudantArgumentError ___ type is not callable:

feed = Feed(self.db, foo={}) # type is dict
feed = Feed(self.db, foo=object_var) # type is object

We should probably only allow int, float, string, and bool types.

@smithsz
Copy link
Contributor Author

smithsz commented Sep 13, 2018

I think we should serialise foo={}. Obviously serialising foo=object() will never work and should throw.

How about making json.dumps the default type converter?

@smithsz smithsz force-pushed the 402-pass-params-to-custom-changes-filter branch from fb490ca to 2c983e6 Compare September 13, 2018 10:10
@smithsz smithsz force-pushed the 402-pass-params-to-custom-changes-filter branch from 2c983e6 to c74b7d8 Compare September 13, 2018 10:21
Copy link
Contributor

@emlaver emlaver left a comment

Choose a reason for hiding this comment

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

Looks good

@smithsz smithsz merged commit 3b1ff13 into master Sep 13, 2018
@smithsz smithsz deleted the 402-pass-params-to-custom-changes-filter branch September 13, 2018 14:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot set filter parameters in changes() call

3 participants