You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by GitBox <gi...@apache.org> on 2022/06/03 22:57:51 UTC

[GitHub] [tinkerpop] simonz-bq opened a new pull request, #1682: Specify ujson version of 2.0.0 or greater

simonz-bq opened a new pull request, #1682:
URL: https://github.com/apache/tinkerpop/pull/1682

   Fixes https://issues.apache.org/jira/browse/TINKERPOP-2631
   
   Since ujson is an optional dependency, hard enforcing a version may cause problems for users.
   
   Instead, a version >= 2.0.0 is specified to be automatically resolved by the Python package manager if someone were to use install gremlin-python with ujson enabled. A warning is also given when using a version below 2.0.0 is detected.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] lyndonbauto commented on pull request #1682: Specify ujson version of 2.0.0 or greater

Posted by GitBox <gi...@apache.org>.
lyndonbauto commented on PR #1682:
URL: https://github.com/apache/tinkerpop/pull/1682#issuecomment-1147741364

   VOTE+1


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] simonz-bq commented on a diff in pull request #1682: Specify ujson version of 2.0.0 or greater

Posted by GitBox <gi...@apache.org>.
simonz-bq commented on code in PR #1682:
URL: https://github.com/apache/tinkerpop/pull/1682#discussion_r890408506


##########
gremlin-python/src/main/python/setup.py:
##########
@@ -79,8 +79,9 @@
         'PyHamcrest>=1.9.0,<2.0.0'
     ],
     install_requires=install_requires,
-    extra_require={
-        'kerberos': 'kerberos>=1.3.0,<2.0.0'    # Does not install in Microsoft Windows
+    extras_require={
+        'kerberos': 'kerberos>=1.3.0,<2.0.0',    # Does not install in Microsoft Windows
+        'ujson': 'ujson>=2.0.0'

Review Comment:
   It's a non-built-in version of json for Python, and was considered optional if the user wanted to use it in their system or they already had the dependency from another part on their system. Putting it in install_require will download the ujson dependency and use it always.
   
   This can't prevent someone from using ujson, but it will show them a warning if they're under version 2. Specifying it here also gives Python developers a place to look, and allows people to use it if they want with `pip install gremlin-python[ujson]`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] lyndonbauto commented on pull request #1682: Specify ujson version of 2.0.0 or greater

Posted by GitBox <gi...@apache.org>.
lyndonbauto commented on PR #1682:
URL: https://github.com/apache/tinkerpop/pull/1682#issuecomment-1152618130

   Thanks @simonz-bq, will merge now with lazy consensus.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] lyndonbauto merged pull request #1682: Specify ujson version of 2.0.0 or greater

Posted by GitBox <gi...@apache.org>.
lyndonbauto merged PR #1682:
URL: https://github.com/apache/tinkerpop/pull/1682


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [tinkerpop] lyndonbauto commented on a diff in pull request #1682: Specify ujson version of 2.0.0 or greater

Posted by GitBox <gi...@apache.org>.
lyndonbauto commented on code in PR #1682:
URL: https://github.com/apache/tinkerpop/pull/1682#discussion_r890359675


##########
gremlin-python/src/main/python/setup.py:
##########
@@ -79,8 +79,9 @@
         'PyHamcrest>=1.9.0,<2.0.0'
     ],
     install_requires=install_requires,
-    extra_require={
-        'kerberos': 'kerberos>=1.3.0,<2.0.0'    # Does not install in Microsoft Windows
+    extras_require={
+        'kerberos': 'kerberos>=1.3.0,<2.0.0',    # Does not install in Microsoft Windows
+        'ujson': 'ujson>=2.0.0'

Review Comment:
   Possibly a stupid question, but why is this in extra_require instead of [install_require](https://github.com/apache/tinkerpop/pull/1682/files#diff-f7172d5670efe2a25eb77a7385821c9e7bfa40ec98b215b8fee4ffae7e0105faR47)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org