Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings
This repository was archived by the owner on Sep 8, 2025. It is now read-only.
/couchdb-pythonPublic archive

Query Server package code review#286

Open
iblislin wants to merge66 commits intodjc:master
base:master
Choose a base branch
Loading
fromiblislin:issue-268

Conversation

iblislin
Copy link

Let's review it commit-by-commit!

Some highlight:

  • The__main__.py will invokeserver.__init__.SimpleQueryServer
  • TheSimpleQueryServer support some legacy query protocol. Are we still need them?

Here is the snippet about the command support.

if (0,9,0)<=self.version< (0,10,0):self.commands['show_doc']=render.show_docself.commands['list_begin']=render.list_beginself.commands['list_row']=render.list_rowself.commands['list_tail']=render.list_tailself.commands['validate']=validate.validateelif (0,10,0)<=self.version< (0,11,0):self.commands['show']=render.showself.commands['list']=render.listself.commands['filter']=filters.filterself.commands['update']=render.updateself.commands['validate']=validate.validateelifself.version>= (0,11,0):ddoc_commands= {}ddoc_commands['shows']=render.ddoc_showddoc_commands['lists']=render.ddoc_listddoc_commands['filters']=filters.ddoc_filterddoc_commands['updates']=render.ddoc_updateddoc_commands['validate_doc_update']=validate.ddoc_validateifself.version>= (1,1,0):self.commands['add_lib']=state.add_libddoc_commands['views']=filters.ddoc_viewsifself.version>= (0,11,0):self.commands['ddoc']=ddoc.DDoc(ddoc_commands)

kxepal added30 commitsMay 9, 2016 23:35
- Add `server/exceptions.py`Author:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <e196819@hotmail.com>- Rename `ViewServerException` to `QueryServerException`Reference:djc#268See Also:djc#276
- in server/__init__.pyAuthor:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>- pep8 coding style checked with  $ pep8 --max-line-length=100 --show-source --firstReference:djc#268See Also:djc#276
Start to work on callable command line interface ofquery server package.Author:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>- Change the entry points of query server in setup.pyReference:djc#268See Also:djc#276
Author:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
Author:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
- We use the `SimpleQueryServer` for `main` entry- Add config handler in `BaseQueryServer`- `BaseQueryServer.serve_forever` API- Test cases for query server includedAuthor:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>- pep8 coding style checked with  $ pep8 --max-line-length=100 --show-source --first- using new style format string -- `str.format`Reference:djc#268See Also:djc#276
Author:     Alexander Shorin <kxepal@gmail.com>Reference:djc#268See Also:djc#276
Author:     Alexander Shorin <kxepal@gmail.com>Reference:djc#268See Also:djc#276
Author:     Alexander Shorin <kxepal@gmail.com>Reference:djc#268See Also:djc#276
Author:     Alexander Shorin <kxepal@gmail.com>Reference:djc#268See Also:djc#276
Author:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <e196819@hotmail.com>- Helper function `_get_db_version`:  doctest includedReference:djc#268See Also:djc#276
- Required by `_process_request`Author:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
- Required by `BaseQueryServer.process_request`Author:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
Author:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
Author:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
The dependency chain:(The "*" mark denote the function/obj included in this commit)compiler.compile_func    |    +- compiler.require    |   |*   |   +- compiler.resolve_module    |   |    |   +- compiler.maybe_export_egg    |   |   |    |   |   +- compiler.maybe_b64egg    |   |   |    |   |   +- compiler.import_b64egg    |   |    |   +- compiler.maybe_export_cached_egg    |   |    |   +- compiler.maybe_compile_function    |   |    |   +- compiler.maybe_export_bytecode    |   |    |   +- compiler.cache_to_ddoc    |    +- compiler.compile_to_bytecode- Test cases includedAuthor:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>- pep8 coding style checked with  $ pep8 --max-line-length=100 --show-source --firstReference:djc#268See Also:djc#276
The dependency chain:(The "*" mark denote the function/obj included in this commit)compiler.compile_func    |    +- compiler.require    |   |    |   +- compiler.resolve_module    |   |*   |   +- compiler.maybe_export_egg*   |   |   |*   |   |   +- compiler.maybe_b64egg*   |   |   |*   |   |   +- compiler.import_b64egg    |   |    |   +- compiler.maybe_export_cached_egg    |   |    |   +- compiler.maybe_compile_function    |   |    |   +- compiler.maybe_export_bytecode    |   |    |   +- compiler.cache_to_ddoc    |    +- compiler.compile_to_bytecodeAuthor:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
Helper functions for `compiler.require`The dependency chain:(The "*" mark denote the function/obj included in this commit)compiler.compile_func    |    +- compiler.require    |   |    |   +- compiler.resolve_module    |   |    |   +- compiler.maybe_export_egg    |   |   |    |   |   +- compiler.maybe_b64egg    |   |   |    |   |   +- compiler.import_b64egg    |   |*   |   +- compiler.maybe_export_cached_egg*   |   |*   |   +- compiler.maybe_compile_function*   |   |*   |   +- compiler.maybe_export_bytecode*   |   |*   |   +- compiler.cache_to_ddoc    |    +- compiler.compile_to_bytecodeAuthor:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
- Test cases includedThe dependency chain:(The "*" mark denote the function/obj included in this commit)compiler.compile_func    |*   +- compiler.require    |   |    |   +- compiler.resolve_module    |   |    |   +- compiler.maybe_export_egg    |   |   |    |   |   +- compiler.maybe_b64egg    |   |   |    |   |   +- compiler.import_b64egg    |   |    |   +- compiler.maybe_export_cached_egg    |   |    |   +- compiler.maybe_compile_function    |   |    |   +- compiler.maybe_export_bytecode    |   |    |   +- compiler.cache_to_ddoc    |    +- compiler.compile_to_bytecodeAuthor:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
-Test cases includedThe dependency chain:(The "*" mark denote the function/obj included in this commit)compiler.compile_func    |    +- compiler.require    |   |    |   +- compiler.resolve_module    |   |    |   +- compiler.maybe_export_egg    |   |   |    |   |   +- compiler.maybe_b64egg    |   |   |    |   |   +- compiler.import_b64egg    |   |    |   +- compiler.maybe_export_cached_egg    |   |    |   +- compiler.maybe_compile_function    |   |    |   +- compiler.maybe_export_bytecode    |   |    |   +- compiler.cache_to_ddoc    |*   +- compiler.compile_to_bytecodeAuthor:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
- Test cases includedAuthor:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
- Test cases includedAuthor:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
- Test cases includedAuthor:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
- Test cases includedAuthor:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
Author:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
- Test cases includedAuthor:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
- Test cases includedAuthor:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
- Test cases includedAuthor:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
Some objects required by `render.show_doc` will be commited later.The dependency chain:render.show_doc    |    +- mime.MimeProvider    |    +- mime.MimeProvider.register_type    |    +- render.response_with    |   |    |   +- mime.MimeProvider.resp_content_type    |    +- render.render_function    |   |    |   +- render.maybe_wrap_responseAuthor:     Alexander Shorin <kxepal@gmail.com>Patched by: Iblis Lin <iblis@hs.ntnu.edu.tw>Reference:djc#268See Also:djc#276
Also, add `render` test suite.The render TestCase require the module `couchdb.server.mock`.render.show_doc    |    +- mime.MimeProvider    |    +- mime.MimeProvider.register_type    |    +- render.response_with    |   |    |   +- mime.MimeProvider.resp_content_type    |*   +- render.render_function*   |   |*   |   +- render.maybe_wrap_responseAuthor:     Alexander Shorin <kxepal@gmail.com>Reference:djc#268See Also:djc#276
else:
msg='Multiple functions are defined. Only one is allowed.'
elifnotisinstance(item,ModuleType):
msg='Only functions could be defined at top level namespace'
Copy link
Author

Choose a reason for hiding this comment

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

I was wandering ... why do we make this limit?

We can simply pop the var fromglobals_ and insert it intocontext, then let user enjoy the global var.
( I think user imported module can be global, also)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Because there is no any sane reason for using anything else except modules and functions in globals. Otherwise users will try to use classes and hit awkward issues because all the design functions should be pure.

Copy link
Author

Choose a reason for hiding this comment

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

If we throw all the function and module(s) into global scope, it will be handy for user. But do we introduce some terrible side effect?

diff --git a/couchdb/server/compiler.py b/couchdb/server/compiler.pyindex c91a4ef..3ba2022 100644--- a/couchdb/server/compiler.py+++ b/couchdb/server/compiler.py@@ -385,4 +385,8 @@ def compile_func(funsrc, ddoc=None, context=None, encoding='utf-8', **options):     if msg is not None:         log.error('%s\n%s', msg, funsrc)         raise Error('compilation_error', msg)++    context.update(globals_)+    globals_.clear()+     return funcdiff --git a/couchdb/tests/server/compiler.py b/couchdb/tests/server/compiler.pyindex ac50b3b..edc0c32 100644--- a/couchdb/tests/server/compiler.py+++ b/couchdb/tests/server/compiler.py@@ -254,19 +259,23 @@ class CompilerTestCase(unittest.TestCase):     def test_allow_imports(self):         funsrc = (             'import math\n'-            'def test(math=math):\n'+            'def test():\n'             '  return math.sqrt(42*42)'         )         func = compiler.compile_func(funsrc)         self.assertEqual(func(), 42)-    def test_fail_for_non_clojured_imports(self):+    def test_for_module_scope(self):         funsrc = (             'import math\n'             'def test():\n'-            '  return math.sqrt(42*42)')+            '  assert "math" in globals()\n'+            '  return math.sqrt(42*42)\n'+            # At the time being executed,+            # we do not move module/func into globals yet+            'assert "math" not in globals()\n'+        )         func = compiler.compile_func(funsrc)-        self.assertRaises(NameError, func)+        self.assertEqual(func(), 42)     def test_ascii_function_source_string(self):         funsrc = 'def test(): return 42'

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, that's the most annoying moment. I don't see any way how it will cause terrible side effect (since we're on Python with a lot of side effects) while we control global scope for each function. I think usability matters more since you don't dodef main(math=math) in your everydays code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Worth to add another assertion that we don't pollute next function compile/call cycle by imports from the previous one.

Copy link
Collaborator

Choose a reason for hiding this comment

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

nice

Copy link
Author

@iblisliniblislinMay 31, 2016
edited
Loading

Choose a reason for hiding this comment

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

So, maybe we should note user not to make some side effect on module. Modules will be cached by Cpython.

Acctually, even if we do not change the scope of module into global, this problem still exists.
Once we are using the same Cpython process, we still have this problem.

In [1]: def f():   ...:     import math   ...:     math.answer = 42   ...:     In [2]: f()In [3]: def g():   ...:     import math   ...:     print(math.answer)   ...:     In [4]: g()42

Copy link
Author

@iblisliniblislinMay 31, 2016
edited
Loading

Choose a reason for hiding this comment

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

I think the patch v1 is fine enougth now, except we really want to take care the module cache( manually GC? ).

diff --git a/couchdb/server/compiler.py b/couchdb/server/compiler.pyindex c91a4ef..3ba2022 100644--- a/couchdb/server/compiler.py+++ b/couchdb/server/compiler.py@@ -385,4 +385,8 @@ def compile_func(funsrc, ddoc=None, context=None, encoding='utf-8', **options):     if msg is not None:         log.error('%s\n%s', msg, funsrc)         raise Error('compilation_error', msg)++    context.update(globals_)+    globals_.clear()+     return funcdiff --git a/couchdb/tests/server/compiler.py b/couchdb/tests/server/compiler.pyindex ac50b3b..edc0c32 100644--- a/couchdb/tests/server/compiler.py+++ b/couchdb/tests/server/compiler.py@@ -254,19 +259,23 @@ class CompilerTestCase(unittest.TestCase):     def test_allow_imports(self):         funsrc = (             'import math\n'-            'def test(math=math):\n'+            'def test():\n'             '  return math.sqrt(42*42)'         )         func = compiler.compile_func(funsrc)         self.assertEqual(func(), 42)-    def test_fail_for_non_clojured_imports(self):+    def test_module_scope(self):         funsrc = (             'import math\n'             'def test():\n'-            '  return math.sqrt(42*42)')+            '  assert "math" in globals()\n'+            '  return math.sqrt(42*42)\n'+            # At the time being executed,+            # we do not move module/func into globals yet+            'assert "math" not in globals()\n'+        )         func = compiler.compile_func(funsrc)-        self.assertRaises(NameError, func)+        self.assertEqual(func(), 42)     def test_ascii_function_source_string(self):         funsrc = 'def test(): return 42'

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, I think we can create issue on this to improve that behaviour. Finally, if you do such blackmagic you should know what are you doing and since we don't provide any sandbox, you shouldn't blindly trust thirdparty code without very pedantic analysis of it.

Copy link
Author

Choose a reason for hiding this comment

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

Ha, forgot 'manaully gc'. That's some kind of joking words, but text message is hard to express it. (Maybe i should add adelete line 😆 )

I will stand for patch v1 and highlight "no sandbox, use side effect at your own risk" on documentation.
Perhaps there are someone really need thishidden feature side effect on module.

compiler.test_required_modules_has_global_namespace_accessReference:djc#268See Also:djc#276
return dedent(getsource(fun))
elif isinstance(fun, util.strbase):
return fun
raise TypeError('Function object or source string expected, got %r' % fun)
Copy link
Author

Choose a reason for hiding this comment

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

Err... It seems that it blew up when user put some builtin function here.

e.g.:

In [20]:sortedOut[20]:<functionsorted>In [21]:type(sorted)Out[21]:builtin_function_or_methodIn [22]:inspect.getsource(sorted)---------------------------------------------------------------------------TypeErrorTraceback (mostrecentcalllast)<ipython-input-22-c9deb9140b13>in<module>()---->1inspect.getsource(sorted)/home/iblis/py/py36/lib/python3.6/inspect.pyingetsource(object)925orcodeobject.Thesourcecodeisreturnedasasinglestring.An926OSErrorisraisedifthesourcecodecannotberetrieved."""--> 927     lines, lnum = getsourcelines(object)    928     return ''.join(lines)    929/home/iblis/py/py36/lib/python3.6/inspect.py in getsourcelines(object)    912     raised if the source code cannot be retrieved."""913object=unwrap(object)-->914lines,lnum=findsource(object)915916ifismodule(object):/home/iblis/py/py36/lib/python3.6/inspect.pyinfindsource(object)725israisedifthesourcecodecannotberetrieved."""    726--> 727     file = getsourcefile(object)    728     if file:    729         # Invalidate cache if needed./home/iblis/py/py36/lib/python3.6/inspect.py in getsourcefile(object)    641     Return None if no way can be identified to get the source.    642     """-->643filename=getfile(object)644all_bytecode_suffixes=importlib.machinery.DEBUG_BYTECODE_SUFFIXES[:]645all_bytecode_suffixes+=importlib.machinery.OPTIMIZED_BYTECODE_SUFFIXES[:]/home/iblis/py/py36/lib/python3.6/inspect.pyingetfile(object)623returnobject.co_filename624raiseTypeError('{!r} is not a module, class, method, '-->625'function, traceback, frame, or code object'.format(object))626627defgetmodulename(path):TypeError:<built-infunctionsorted>isnotamodule,class,method,function,traceback,frame,orcodeobject

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I know. Same story with lambdas. Basically, both are not suitable for any kind of design function, so no reason to care about.

iblislin reacted with thumbs up emoji
@iblislin
Copy link
Author

I'm going to attendPyCon TW this weekend.

Maybe disappear about 3 days....

@kxepal
Copy link
Collaborator

Cool! Have fun there! (:

iblislin reacted with laugh emoji

server.state['query_config'].update(config)
ifserver.version>= (1,1,0):
server.state['view_lib']=''
returnTrue
Copy link
Author

Choose a reason for hiding this comment

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

We initserver.state in the constructor ofBaseQueryServer.

self._state= {'view_lib':None,'line_length':0,'query_config': {},'functions': [],'functions_src': [],'row_line': {}        }

But in thisreset function, we do not reset the value ofline_length androw_line.

Aboutline_length: I cannot find any function need it. Is this variable an unimplemented feature?

Followinggrep shows that only docstring mensioned it.
It's the partner ofreduce_limilt, but, thereduce_limit cannot be configured from commandline optoin.
Isreduce_limit an unimplemented feature, also?

└─[iblis@imfsa]% grep -1 line_length*__init__.py-'view_lib': None,__init__.py:'line_length': 0,__init__.py-'query_config': {},Binary file __init__.pyc matches--views.py-          If any Python exception occurs or reduce output is twice longerviews.py:          as state.line_length and reduce_limit is enabledin state.query_configviews.py-"""--views.py-          If any Python exception occurs or reduce output is twice longerviews.py:          as state.line_length and reduce_limit is enabled in state.query_configviews.py-"""Binary file views.pyc matches

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's an artefact from JS query server backport.It has line_length value in state, but we eventually don't use it. Instead wehave it used differently without using state.

Copy link
Author

Choose a reason for hiding this comment

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

👍 . This patch removed thestate.line_length and change the docstring ofreduce andrereduce

diff --git a/couchdb/server/__init__.py b/couchdb/server/__init__.pyindex 40a4328..f72e855 100644--- a/couchdb/server/__init__.py+++ b/couchdb/server/__init__.py@@ -51,7 +51,6 @@ class BaseQueryServer(object):         self._config = {}         self._state = {             'view_lib': None,-            'line_length': 0,             'query_config': {},             'functions': [],             'functions_src': [],diff --git a/couchdb/server/views.py b/couchdb/server/views.pyindex e885c65..6ce7ac3 100644--- a/couchdb/server/views.py+++ b/couchdb/server/views.py@@ -79,7 +79,8 @@ def reduce(server, reduce_funs, kvs, rereduce=False):     :raises:         - :exc:`~couchdb.server.exceptions.Error`           If any Python exception occurs or reduce output is twice longer-          as state.line_length and reduce_limit is enabled in state.query_config+          than input key-value pairs.+          In the latter case, we consider it as ``reduce_overflow_error``.     """     reductions = []     _append = reductions.append@@ -132,7 +133,8 @@ def rereduce(server, reduce_funs, values):     :raises:         - :exc:`~couchdb.server.exceptions.Error`           If any Python exception occurs or reduce output is twice longer-          as state.line_length and reduce_limit is enabled in state.query_config+          than input key-value pairs.+          In the latter case, we consider it as ``reduce_overflow_error``.     """     log.debug('Rereducing values:\n%s', values)     return reduce(server, reduce_funs, values, rereduce=True)

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 Nice catch!

iblislin reacted with thumbs up emoji
Copy link
Author

Choose a reason for hiding this comment

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

Hmm..next issues: Do we need to expose thereduce_limilt to command line? If yes, what do we expect when thereset is invoked?.(reset cleanup thestate['query_config'], where thereduce_limit live.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Reset is a command which server sends to QS. Mostly everytime when it tries to execute a view.

Copy link
Author

Choose a reason for hiding this comment

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

Yep. The main concern in my question is that "Ifstate get reset again and again, the value ofreduce_limilt will be volatile. Should this value be basically consist?"
I guessreduce_limit do not change in most of the time, so let user set it via command line option (just a boolean flag) make more sense to me.

FYI:
I am looking this code block inBaseQueryServer

defis_reduce_limited(self):"""Checks if output of reduce function is limited."""returnself.state['query_config'].get('reduce_limit',False)

Copy link
Collaborator

@kxepalkxepalJun 3, 2016
edited
Loading

Choose a reason for hiding this comment

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

Well, this option also comes from CouchDB with the reset command (:
http://docs.couchdb.org/en/1.6.1/query-server/protocol.html#reset
So no reason to care about - that's not something we control.

iblislin reacted with thumbs up emoji
Copy link
Author

Choose a reason for hiding this comment

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

Ha! Ok, i miss the doc 😄

Copy link
Author

Choose a reason for hiding this comment

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

The patch aboutstate.line_length was committed @a2ff50d.

server.state['query_config']['foo']='bar'
state.reset(server, {'foo':'baz'})
self.assertEqual(server.state['query_config'], {'foo':'baz'})

Copy link
Author

Choose a reason for hiding this comment

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

Add test case about resettingview_lib.

diff --git a/couchdb/server/state.py b/couchdb/server/state.pyindex ee0ee0e..8cf8617 100644--- a/couchdb/server/state.py+++ b/couchdb/server/state.py@@ -29,7 +29,7 @@ def reset(server, config=None):         log.debug('Set new query config:\n%s', config)         server.state['query_config'].update(config)     if server.version >= (1, 1, 0):-        server.state['view_lib'] = ''+        server.state['view_lib'] = None     return Truediff --git a/couchdb/tests/server/state.py b/couchdb/tests/server/state.pyindex 42d7f52..71ad40a 100644--- a/couchdb/tests/server/state.py+++ b/couchdb/tests/server/state.py@@ -64,6 +64,13 @@ class StateTestCase(unittest.TestCase):         state.reset(server, {'foo': 'baz'})         self.assertEqual(server.state['query_config'], {'foo': 'baz'})+    def test_reset_view_lib(self):+        server = MockQueryServer(version=(1, 1, 0))+        self.assertEqual(server.state['view_lib'], None)+        server.state['view_lib'] = {'foo': 'bar'}+        state.reset(server)+        self.assertEqual(server.state['view_lib'], None)+ def suite():     suite = unittest.TestSuite()

- The output is key-value pair only- Also, provide comprehensive messageReference:djc#268See Also:djc#276
@iblislin
Copy link
Author

I have to get through my final exam 😭
I will be back about 1 week later...

' pass'
)
doc = {'_id': 'foo'}
views.map_doc(self.server, doc)
Copy link
Author

Choose a reason for hiding this comment

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

Good to check output ofmap_doc?

diff --git a/couchdb/tests/server/views.py b/couchdb/tests/server/views.pyindex 0788d4c..7751487 100644--- a/couchdb/tests/server/views.py+++ b/couchdb/tests/server/views.py@@ -115,7 +115,7 @@ class MapTestCase(unittest.TestCase):             '  pass'         )         doc = {'_id': 'foo'}-        views.map_doc(self.server, doc)+        self.assertEqual(views.map_doc(self.server, doc), [[]])     def test_yield_non_iterable(self):         """should raise Error if map function do not yield iterable"""

FYI: the second example inhttp://docs.couchdb.org/en/1.6.1/query-server/protocol.html#map-doc

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

Copy link
Author

Choose a reason for hiding this comment

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

comitted @f91a237

Copy link
Author

Choose a reason for hiding this comment

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

have no idea why the build failed ...

Copy link
Collaborator

Choose a reason for hiding this comment

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

How?

Copy link
Author

Choose a reason for hiding this comment

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

I add the verbose flag but the error gone.
https://travis-ci.org/iblis17/couchdb-python/builds/139946573
It seems that it failed randomly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Heisenbug! Wonder what caused it since we don't do any blocking operations in the tests. May be something travis specific? I remember I had a test ran for 2 days on travis (not a joke) being hanged. There wasn't bug in code however.

Copy link
Author

Choose a reason for hiding this comment

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

😆 Heisenbug.
I am trying to press rebuild button again and again now...

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice catch!

======================================================================FAIL: test_timeout_retry (couchdb.tests.couchhttp.SessionTestCase)----------------------------------------------------------------------Traceback (most recent call last):  File "/home/travis/build/iblis17/couchdb-python/couchdb/tests/couchhttp.py", line 32, in test_timeout_retry    self.assertRaises(socket.timeout, session.request, 'GET', db.resource.url)AssertionError: timeout not raised by request----------------------------------------------------------------------

but seems like ENOTOURPROBLEM.

@djcdjc mentioned this pull requestAug 5, 2016
@iblisliniblislinforce-pushed theissue-268 branch 2 times, most recently frome71e522 toea10798CompareOctober 25, 2016 01:30
# <http://www.iana.org/assignments/media-types/>`_ for HTTP responses.
# Ported from `Ruby on Rails
# <https://github.com/rails/rails/blob/v3.1.0/actionpack/lib/action_dispatch/http/mime_types.rb>`_
DEFAULT_TYPES= {
Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good idea!

'all': ['*/*'],
'text': ['text/plain; charset=utf-8','txt'],
'html': ['text/html; charset=utf-8'],
'xhtml': ['application/xhtml+xml','xhtml'],
Copy link
Author

Choose a reason for hiding this comment

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

'xhtml': ['application/xhtml+xml','xhtml'],^^^^^^whyduplicate`xhtml`here?

And in case oftext

'text': ['text/plain; charset=utf-8','txt'],

I guess it should be

- 'text': ['text/plain; charset=utf-8', 'txt'],+ 'text': ['text/plain; charset=utf-8'],+ 'txt': ['text/plain; charset=utf-8'],

makeMimeProvider.register_type create two entries ofmimes_by_key ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Author

@iblisliniblislinOct 25, 2016
edited
Loading

Choose a reason for hiding this comment

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

OMG... i am going to open ticket on jira ( but stuck on jira's login page, no idea why login failed even i changed password again and again

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good idea. May be JIRA is on maintenance or something? Have you change your keyboard layout before typing a password?

Copy link
Author

Choose a reason for hiding this comment

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

hmm,i just reset my password again... now encounterthis. And i changed the browser... but still cannot pass captcha. 😭

Copy link
Author

Choose a reason for hiding this comment

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

JIRA isn't on maintenance...

Copy link
Author

Choose a reason for hiding this comment

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

Maybe I should create another account on jira...


Patch

  • remove duplicatexhtml
  • introduce type aliases forDEFAULT_TYPES, e.g.
    ('text', 'txt'): ['text/plain; charset=utf-8'],.txt is an alias oftext.
diff --git a/couchdb/server/mime.py b/couchdb/server/mime.pyindex 7e185d5..767c809 100644--- a/couchdb/server/mime.py+++ b/couchdb/server/mime.py@@ -78,26 +78,35 @@ def best_match(supported, header): # Build list of `MIME types # <http://www.iana.org/assignments/media-types/>`_ for HTTP responses. # Ported from `Ruby on Rails-# <https://github.com/rails/rails/blob/v3.1.0/actionpack/lib/action_dispatch/http/mime_types.rb>`_+# <https://github.com/rails/rails/blob/v5.0.0.1/actionpack/lib/action_dispatch/http/mime_types.rb>`_ DEFAULT_TYPES = {-    'all': ['*/*'],-    'text': ['text/plain; charset=utf-8', 'txt'],-    'html': ['text/html; charset=utf-8'],-    'xhtml': ['application/xhtml+xml', 'xhtml'],-    'xml': ['application/xml', 'text/xml', 'application/x-xml'],-    'js': ['text/javascript', 'application/javascript',-           'application/x-javascript'],-    'css': ['text/css'],-    'ics': ['text/calendar'],-    'csv': ['text/csv'],-    'rss': ['application/rss+xml'],-    'atom': ['application/atom+xml'],-    'yaml': ['application/x-yaml', 'text/yaml'],+    # (type, alias, ...): [content_type, ...]+    ('all',): ['*/*'],+    ('text', 'txt'): ['text/plain; charset=utf-8'],+    ('html',): ['text/html; charset=utf-8'],+    ('xhtml',): ['application/xhtml+xml'],+    ('js',): ['text/javascript',+              'application/javascript',+              'application/x-javascript'],+    ('css',): ['text/css'],+    ('ics',): ['text/calendar'],+    ('csv',): ['text/csv'],+    ('vcf',): ['text/vcard'],++    ('xml',): ['application/xml', 'text/xml', 'application/x-xml'],+    ('rss',): ['application/rss+xml'],+    ('atom',): ['application/atom+xml'],+    ('yaml', 'yml'): ['application/x-yaml', 'text/yaml'],     # just like Rails-    'multipart_form': ['multipart/form-data'],-    'url_encoded_form': ['application/x-www-form-urlencoded'],+    ('multipart_form',): ['multipart/form-data'],+    ('url_encoded_form',): ['application/x-www-form-urlencoded'],     # http://www.ietf.org/rfc/rfc4627.txt-    'json': ['application/json', 'text/x-json']+    # http://www.json.org/JSONRequest.html+    ('json',): ['application/json', 'text/x-json', 'application/jsonrequest'],++    ('pdf',): ['application/pdf'],+    ('zip',): ['application/zip'],+    ('gzip', 'gz'): ['application/gzip'],     # TODO: https://issues.apache.org/jira/browse/COUCHDB-1261     # 'kml', 'application/vnd.google-earth.kml+xml',     # 'kmz', 'application/vnd.google-earth.kmz'@@ -113,8 +122,9 @@ class MimeProvider(object):         self.funcs_by_key = OrderedDict()         self._resp_content_type = None-        for k, v in DEFAULT_TYPES.items():-            self.register_type(k, *v)+        for types, v in DEFAULT_TYPES.items():+            for type_ in types:+                self.register_type(type_, *v)     def is_provides_used(self):         """Checks if any provides function is registered."""@@ -135,21 +145,33 @@ class MimeProvider(object):         Predefined types:             - all: ``*/*``-            - text: ``text/plain; charset=utf-8``, ``txt``+            - text: ``text/plain; charset=utf-8``+            - txt: alias of ``text``             - html: ``text/html; charset=utf-8``-            - xhtml: ``application/xhtml+xml``, ``xhtml``-            - xml: ``application/xml``, ``text/xml``, ``application/x-xml``+            - xhtml: ``application/xhtml+xml``             - js: ``text/javascript``, ``application/javascript``,               ``application/x-javascript``             - css: ``text/css``             - ics: ``text/calendar``             - csv: ``text/csv``+            - vcf: ``text/vcard``,++            - xml: ``application/xml``, ``text/xml``, ``application/x-xml``             - rss: ``application/rss+xml``             - atom: ``application/atom+xml``             - yaml: ``application/x-yaml``, ``text/yaml``+            - yml: alias of ``yaml``+             - multipart_form: ``multipart/form-data``             - url_encoded_form: ``application/x-www-form-urlencoded``-            - json: ``application/json``, ``text/x-json``++            - json: ``application/json``, ``text/x-json``,+              ``application/jsonrequest``++            - pdf: ``application/pdf``+            - zip: ``application/zip``+            - gzip: ``application/gzip``+            - gz: alias of ``gzip``         Example:             >>> register_type('png', 'image/png')

kxepal reacted with thumbs up emoji
Copy link
Collaborator

Choose a reason for hiding this comment

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

LGFM

Copy link
Author

@iblisliniblislinOct 26, 2016
edited
Loading

Choose a reason for hiding this comment

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

Patched @d099408

(TODO: open ticket on JIRA

- Remove duplicate xhtml- Introduce type aliases for `DEFAULT_TYPES`,  e.g. `('text', 'txt'): ['text/plain; charset=utf-8']`.  `txt` is an alias of `text`.Reference:djc#268See Also:djc#276
returnfitness_and_quality(mimetype,ranges)


defbest_match(supported,header):
Copy link
Author

Choose a reason for hiding this comment

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

How about leveraging standard lib ?
https://docs.python.org/3.7/library/difflib.html#difflib.get_close_matches
or some similar function.

'all','atom','css','csv','html','ics','js','json',
'multipart_form','rss','text','url_encoded_form','xhtml',
'xml','yaml'])
)
Copy link
Author

Choose a reason for hiding this comment

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

Oops, forgot to update test case.

Patch:

diff --git a/couchdb/tests/server/mime.py b/couchdb/tests/server/mime.pyindex 463aa9b..6587f78 100644--- a/couchdb/tests/server/mime.py+++ b/couchdb/tests/server/mime.py@@ -154,9 +154,10 @@ class ProvidesTestCase(MimeTestCase):         self.assertEqual(             sorted(self.provider.mimes_by_key.keys()),             sorted([-                'all', 'atom', 'css', 'csv', 'html', 'ics', 'js', 'json',-                'multipart_form', 'rss', 'text', 'url_encoded_form', 'xhtml',-                'xml', 'yaml'])+                'all', 'atom', 'css', 'csv', 'gz', 'gzip', 'html', 'ics',+                'js', 'json', 'multipart_form', 'pdf', 'rss', 'text', 'txt',+                'url_encoded_form', 'vcf', 'xhtml', 'xml', 'yaml', 'yml',+                'zip'])         )     def test_provides(self):

Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

1 more reviewer

@kxepalkxepalkxepal left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@iblislin@kxepal

[8]ページ先頭

©2009-2025 Movatter.jp