Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

parse filters json only if filters are present #1919

Closed
wants to merge 1 commit into from

Conversation

asmacdo
Copy link
Contributor

@asmacdo asmacdo commented Jun 12, 2015

…ches

fixes: 1040

https://pulp.plan.io/issues/1040

@asmacdo asmacdo force-pushed the 1040 branch 3 times, most recently from cfea091 to 8ec959f Compare June 12, 2015 16:40
@asmacdo asmacdo changed the title parsed args are no longer lists and filters are required for get sear… filters are required for get searches Jun 12, 2015
mock_parse.return_value = ('mock', 'tuple')
search_view = search.SearchView()
mock_request = mock.MagicMock()
mock_request.GET = {}
Copy link
Contributor

Choose a reason for hiding this comment

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

subtle difference, but I'd use a QueryDict here, because why not?

@mhrivnak
Copy link
Contributor

I'm not sure this is the right solution. I can't find that we document anywhere that this is required, but I can think of ways this API is useful without using filters (pagination for example). If you do think it's a good idea to make this a requirement, please elaborate on why.

@mhrivnak mhrivnak assigned mhrivnak and asmacdo and unassigned mhrivnak Jun 12, 2015
@asmacdo asmacdo force-pushed the 1040 branch 2 times, most recently from cfd700f to 0b31b57 Compare June 15, 2015 14:23
@asmacdo asmacdo changed the title filters are required for get searches parse filters json only if filters are present Jun 15, 2015
@asmacdo
Copy link
Contributor Author

asmacdo commented Jun 15, 2015

@mhrivnak, I agree with you. updated.

@asmacdo asmacdo assigned mhrivnak and unassigned asmacdo Jun 15, 2015
@asmacdo
Copy link
Contributor Author

asmacdo commented Jun 15, 2015

ok test

@pulpbot
Copy link
Member

pulpbot commented Jun 15, 2015

Refer to this link for build results (access rights to CI server needed):
https://pulp-jenkins.rhev-ci-vms.eng.rdu2.redhat.com//job/unittest-pulp-pr/913/

Build result: FAILURE

[...truncated 7 lines...]Fetching upstream changes from https://github.com/pulp/pulp.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/pulp/pulp.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse refs/remotes/origin/pr/1919/merge^{commit} # timeout=10 > git rev-parse refs/remotes/origin/origin/pr/1919/merge^{commit} # timeout=10Checking out Revision ec87bdb (refs/remotes/origin/pr/1919/merge) > git config core.sparsecheckout # timeout=10 > git checkout -f ec87bdb > git rev-list ec87bdb # timeout=10Triggering rhel5-npTriggering f20-npTriggering rhel7-npTriggering rhel6-npTriggering f21-nprhel5-np completed with result SUCCESSf20-np completed with result FAILURErhel7-np completed with result SUCCESSrhel6-np completed with result SUCCESSf21-np completed with result FAILUREStarted calculate disk usage of buildFinished Calculation of disk usage of build in 0 secondsStarted calculate disk usage of workspaceFinished Calculation of disk usage of workspace in 3 secondSetting status of 023c106 to FAILURE with url https://pulp-jenkins.rhev-ci-vms.eng.rdu2.redhat.com/job/unittest-pulp-pr/913/ and message: Merged build finished.
Test FAILed.

@asmacdo
Copy link
Contributor Author

asmacdo commented Jun 15, 2015

ok test

mock_parse.return_value = ({'mock': 'query'}, 'tuple')
search_view = search.SearchView()
mock_request = mock.MagicMock()
mock_request.GET = {'filters': 'invalid json'}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd use a QueryDict here for authenticity. It might not make much difference, but we may as well keep tests as close to real circumstances as we can.

@mhrivnak mhrivnak added the LGTM label Jun 15, 2015
@mhrivnak mhrivnak assigned asmacdo and unassigned mhrivnak Jun 15, 2015
@pulpbot
Copy link
Member

pulpbot commented Jun 15, 2015

Refer to this link for build results (access rights to CI server needed):
https://pulp-jenkins.rhev-ci-vms.eng.rdu2.redhat.com//job/unittest-pulp-pr/914/

Build result: FAILURE

[...truncated 7 lines...]Fetching upstream changes from https://github.com/pulp/pulp.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/pulp/pulp.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse refs/remotes/origin/pr/1919/merge^{commit} # timeout=10 > git rev-parse refs/remotes/origin/origin/pr/1919/merge^{commit} # timeout=10Checking out Revision ec87bdb (refs/remotes/origin/pr/1919/merge) > git config core.sparsecheckout # timeout=10 > git checkout -f ec87bdb > git rev-list ec87bdb # timeout=10Triggering rhel5-npTriggering f20-npTriggering rhel7-npTriggering rhel6-npTriggering f21-nprhel5-np completed with result SUCCESSf20-np completed with result FAILURErhel7-np completed with result SUCCESSrhel6-np completed with result SUCCESSf21-np completed with result FAILUREStarted calculate disk usage of buildFinished Calculation of disk usage of build in 0 secondsStarted calculate disk usage of workspaceFinished Calculation of disk usage of workspace in 2 secondSetting status of 023c106 to FAILURE with url https://pulp-jenkins.rhev-ci-vms.eng.rdu2.redhat.com/job/unittest-pulp-pr/914/ and message: Merged build finished.
Test FAILed.

@asmacdo
Copy link
Contributor Author

asmacdo commented Jun 15, 2015

moving to 2.7-testing #1923

@asmacdo asmacdo closed this Jun 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants