anon-iclr-submission/benchname-bug-localization huggingface dataset:
Repository: nvidia/nvflare
Issue: run `pip install nvflare==2.1.2` brings wrong `protobuf` version

When using `pip install nvflare==2.1.2`, protobuf 4.21.4 is installed:
```
Collecting protobuf<5.0.0dev,>=3.15.0
  Downloading protobuf-4.21.4-cp37-abi3-manylinux2014_x86_64.whl (408 kB)
```
and when starting the server (`./poc/server/startup/start.sh`), I got the following error:
```
TypeError: Descriptors cannot not be created directly.
If this call came from a _pb2.py file, your generated code is out of date and must be regenerated with protoc >= 3.19.0.
If you cannot immediately regenerate your protos, some other possible workarounds are:
 1. Downgrade the protobuf package to 3.20.x or lower.
 2. Set PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python (but this will use pure-Python parsing and will be much slower).
```
downgrade into 3.20.1 solves the issue:
```
 pip install protobuf==3.20.1
```
In the `requirements-min.txt` of v2.1.2, it specifies to use `protobuf==3.20.1` (https://github.com/NVIDIA/NVFlare/blob/2.1.2/requirements-min.txt#L7), but why this requirement does not work when running the pip install command? Is there anything wrong here?


Changed files:
setup.py

diff --git a/setup.py b/setup.py
index 67f509df..58ea808a 100644
--- a/setup.py
+++ b/setup.py
@@ -12,9 +12,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import datetime
 import os
 import shutil
-import datetime
 
 from setuptools import find_packages, setup
 
@@ -73,15 +73,17 @@ setup(
     long_description_content_type="text/markdown",
     python_requires=">=3.7,<3.9",
     install_requires=[
-        "PyYAML",
-        "psutil",
+        "PyYAML==6.0",
+        "psutil==5.9.1",
         "numpy",
-        "grpcio",
-        "google-api-python-client",
+        "grpcio==1.46.3",
+        "protobuf==3.20.1",
+        "google-api-python-client==2.49.0",
         "cryptography>=36.0.0",
+        "Flask==2.1.2",
+        "gunicorn==20.1.0",
+        "six>=1.15.0",
         "tenseal==0.3.0",
-        "gunicorn",
-        "flask",
     ],
     entry_points={
         "console_scripts": [
Repository: girder/girder
Issue: Thumbnails: support thumbnailing PNGs in palette mode

Right now this causes a rather mysterious error message: `IOError: cannot write mode P as JPEG`


Changed files:
plugins/thumbnails/server/worker.py

diff --git a/plugins/thumbnails/server/worker.py b/plugins/thumbnails/server/worker.py
index 3bf935208..2ddb71850 100644
--- a/plugins/thumbnails/server/worker.py
+++ b/plugins/thumbnails/server/worker.py
@@ -108,7 +108,7 @@ def createThumbnail(width, height, crop, fileId, attachToType, attachToId):
     uploadModel = ModelImporter.model('upload')
 
     out = six.BytesIO()
-    image.save(out, 'JPEG', quality=85)
+    image.convert('RGB').save(out, 'JPEG', quality=85)
     size = out.tell()
     out.seek(0)
Repository: girder/girder
Issue: Permission control on collections

I thought this was there but I can't find it anywhere in the current build. Even when the user has admin rights to the collection, I don't see a lock button or permissions menu item.


Changed files:
tests/cases/collection_test.py, girder/api/v1/collection.py

diff --git a/girder/api/v1/collection.py b/girder/api/v1/collection.py
index 54eb2b027..0c6c3cbcb 100644
--- a/girder/api/v1/collection.py
+++ b/girder/api/v1/collection.py
@@ -88,7 +88,7 @@ class Collection(Resource):
 
     @loadmodel(map={'id': 'coll'}, model='collection', level=AccessType.READ)
     def getCollection(self, coll, params):
-        return self.model('collection').filter(coll)
+        return self.model('collection').filter(coll, self.getCurrentUser())
     getCollection.description = (
         Description('Get a collection by ID.')
         .responseClass('Collection')
diff --git a/tests/cases/collection_test.py b/tests/cases/collection_test.py
index 8a4c8d5c2..987844e57 100644
--- a/tests/cases/collection_test.py
+++ b/tests/cases/collection_test.py
@@ -111,6 +111,12 @@ class CollectionTestCase(base.TestCase):
         self.assertEqual(resp.json[0]['_id'], newCollId)
         self.assertEqual(resp.json[0]['name'], 'New collection')
 
+        # Test collection get
+        resp = self.request(path='/collection/{}'.format(newCollId),
+                            user=self.admin)
+        self.assertStatusOk(resp)
+        self.assertEqual(resp.json['_accessLevel'], AccessType.ADMIN)
+
     def testDeleteCollection(self):
         # Requesting with no path should fail
         resp = self.request(path='/collection', method='DELETE',
Repository: girder/girder
Issue: Job logs vanish when job status changes

When watching a job in the UI, when the status changes, the logs vanish.  The logs are shown again when you navigate away and back to the job.  This appears to be caused by the `job_status` notification having an empty list for the `log` parameter.


Changed files:
plugins/jobs/plugin_tests/jobs_test.py, plugins/jobs/server/models/job.py

diff --git a/plugins/jobs/plugin_tests/jobs_test.py b/plugins/jobs/plugin_tests/jobs_test.py
index 7b82b5bd2..955c6ef1c 100644
--- a/plugins/jobs/plugin_tests/jobs_test.py
+++ b/plugins/jobs/plugin_tests/jobs_test.py
@@ -289,7 +289,8 @@ class JobsTestCase(base.TestCase):
         self.assertEqual(statusNotify['type'], 'job_status')
         self.assertEqual(statusNotify['data']['_id'], str(job['_id']))
         self.assertEqual(int(statusNotify['data']['status']), JobStatus.ERROR)
-        self.assertTrue('kwargs' not in statusNotify['data'])
+        self.assertNotIn('kwargs', statusNotify['data'])
+        self.assertNotIn('log', statusNotify['data'])
 
         self.assertEqual(progressNotify['type'], 'progress')
         self.assertEqual(progressNotify['data']['title'], job['title'])
diff --git a/plugins/jobs/server/models/job.py b/plugins/jobs/server/models/job.py
index 960020814..e9a857ee9 100644
--- a/plugins/jobs/server/models/job.py
+++ b/plugins/jobs/server/models/job.py
@@ -368,6 +368,7 @@ class Job(AccessControlledModel):
                 expires = now + datetime.timedelta(seconds=30)
                 filtered = self.filter(job, user)
                 filtered.pop('kwargs', None)
+                filtered.pop('log', None)
                 self.model('notification').createNotification(
                     type='job_status', data=filtered, user=user, expires=expires)
Repository: girder/girder
Issue: Correctly handle range requests from video players

> If girder is asked for the whole file (Range 0-), then it responds with a 200.  Reading https://developer.mozilla.org/en-US/docs/Web/HTTP/Range_requests, we probably should still send a 206.

Changed files:
test/test_file.py, girder/utility/abstract_assetstore_adapter.py

diff --git a/girder/utility/abstract_assetstore_adapter.py b/girder/utility/abstract_assetstore_adapter.py
index 5d904c2f5..f63aea165 100644
--- a/girder/utility/abstract_assetstore_adapter.py
+++ b/girder/utility/abstract_assetstore_adapter.py
@@ -3,6 +3,7 @@ import os
 import re
 import six
 
+import cherrypy
 from cherrypy._cpreqbody import Part
 
 from girder.api.rest import setResponseHeader, setContentDisposition
@@ -327,13 +328,14 @@ class AbstractAssetstoreAdapter(object):
             be set to 'attachment; filename=$filename'.
         :type contentDisposition: str or None
         """
+        isRangeRequest = cherrypy.request.headers.get('Range')
         setResponseHeader(
             'Content-Type',
             file.get('mimeType') or 'application/octet-stream')
         setContentDisposition(file['name'], contentDisposition or 'attachment')
         setResponseHeader('Content-Length', max(endByte - offset, 0))
 
-        if (offset or endByte < file['size']) and file['size']:
+        if (offset or endByte < file['size'] or isRangeRequest) and file['size']:
             setResponseHeader(
                 'Content-Range',
                 'bytes %d-%d/%d' % (offset, endByte - 1, file['size']))
diff --git a/test/test_file.py b/test/test_file.py
index 710dcf07c..4ec5323ab 100644
--- a/test/test_file.py
+++ b/test/test_file.py
@@ -1,6 +1,9 @@
 from girder.models.file import File
 from girder.models.folder import Folder
 from girder.models.upload import Upload
+import pytest
+from pytest_girder.assertions import assertStatus
+from pytest_girder.utils import uploadFile
 from six import BytesIO
 
 
@@ -9,3 +12,22 @@ def testEmptyUploadFromFile(admin, fsAssetstore):
     file = Upload().uploadFromFile(BytesIO(b''), size=0, name='empty', parent=dest, user=admin)
     assert File().load(file['_id'], force=True) is not None
     assert file['assetstoreId'] == fsAssetstore['_id']
+
+
+@pytest.mark.parametrize('range,status,cr,cl', (
+    ('bytes=0-', 206, 'bytes 0-99/100', 100),
+    ('bytes=0-10', 206, 'bytes 0-10/100', 11),
+    (None, 200, None, 100)
+))
+def testRangeRequestBehavior(range, status, cr, cl, admin, server, fsAssetstore):
+    dest = Folder().childFolders(admin, parentType='user')[0]
+    file = uploadFile('test', b'a' * 100, admin, dest)
+    headers = [('Range', range)] if range else []
+    resp = server.request(
+        path='/file/%s/download' % file['_id'], user=admin, additionalHeaders=headers, isJson=False)
+    assertStatus(resp, status)
+    assert resp.headers['Content-Length'] == cl
+    if cr is None:
+        assert 'Content-Range' not in resp.headers
+    else:
+        assert resp.headers['Content-Range'] == cr
Repository: girder/girder
Issue: Make Girder in cherrypy respond appropriately to SIGTERM

Right now `SIGTERM` sent to `girder serve` does nothing. According to [these cherrypy docs](https://docs.cherrypy.org/en/latest/pkg/cherrypy.process.html#cherrypy.process.plugins.SignalHandler), cherrypy should exit gracefully when SIGTERM is sent, but it doesn't. We should figure out why that is. Are we overriding these defaults somehow? Or is this an upstream bug?

Changed files:
girder/cli/serve.py

diff --git a/girder/cli/serve.py b/girder/cli/serve.py
index 3648b687b..4f12ee350 100644
--- a/girder/cli/serve.py
+++ b/girder/cli/serve.py
@@ -34,5 +34,6 @@ def main(dev, mode, database, host, port):
     _attachFileLogHandlers()
     server.setup(mode)
 
+    cherrypy.engine.signal_handler.subscribe()
     cherrypy.engine.start()
     cherrypy.engine.block()
Repository: girder/girder
Issue: Settings cache leads to unexpected behavior

If you've enable the dogpile setting cache and have an event listener on `model.setting.save.after`, the event will get the old value of the setting, not the new value.

Specifically, if you configure the cache via the ini file like:
```
[cache]
enabled: True
cache.global.backend = "dogpile.cache.memory"
```
and, in some code, bind to the settings change:
```
events.bind('model.setting.save.after', 'cool.plugin', onSettingChange)
```
Then in the function `onSettingChange`, `Setting().get(<setting name>)`, will get the old value.  The event has the new value.

Within the setting model, this could be prevented by invalidating the cache before the save (e.g., change:
```
        setting = self.save(setting)
        self._get.set(setting, self, key)
```
to:
```
        self._get.invalidate(self, key)
        setting = self.save(setting)
        self._get.set(setting, self, key)
```
though there will still be race conditions if multiple threads are changing the setting concurrently.

Changed files:
girder/models/setting.py

diff --git a/girder/models/setting.py b/girder/models/setting.py
index dadfc0539..4987d58db 100644
--- a/girder/models/setting.py
+++ b/girder/models/setting.py
@@ -118,6 +118,10 @@ class Setting(Model):
         else:
             setting['value'] = value
 
+        # Invalidate the cache so that events that listen for a change in the
+        # setting will be get the right value from a get.  We don't set it here
+        # as it could fail to validate and save.
+        self._get.invalidate(self, key)
         setting = self.save(setting)
 
         self._get.set(setting, self, key)
Repository: girder/girder
Issue: girder-client --certificate option doesn't work with --api-url

This comes from the following check in `cli.py`:

```python
    if certificate and ctx.obj.scheme != 'https':
        raise click.BadArgumentUsage(
            'A URI scheme of "https" is required for option "--certificate"')
```

Most of the time we don't pass the scheme, we use `--api-url` instead.

Changed files:
clients/python/girder_client/cli.py

diff --git a/clients/python/girder_client/cli.py b/clients/python/girder_client/cli.py
index 7a2a501f7..49b5b872e 100755
--- a/clients/python/girder_client/cli.py
+++ b/clients/python/girder_client/cli.py
@@ -159,8 +159,8 @@ _CONTEXT_SETTINGS = dict(help_option_names=['-h', '--help'])
               show_default=True,
               cls=_AdvancedOption
               )
-@click.option('--certificate', default=None,
-              help='Specify path to SSL certificate',
+@click.option('--ca-certificate', default=None,
+              help='Specify path to CA certificate to use to verify the server',
               show_default=True,
               cls=_AdvancedOption
               )
@@ -175,7 +175,7 @@ _CONTEXT_SETTINGS = dict(help_option_names=['-h', '--help'])
 @click.pass_context
 def main(ctx, username, password,
          api_key, api_url, scheme, host, port, api_root,
-         no_ssl_verify, certificate, token, retries, verbose):
+         no_ssl_verify, ca_certificate, token, retries, verbose):
     """Perform common Girder CLI operations.
 
     The CLI is particularly suited to upload (or download) large, nested
@@ -200,13 +200,13 @@ def main(ctx, username, password,
             raise click.BadArgumentUsage(
                 'Option "--api-url" and option "--%s" are mutually exclusive.' %
                 name.replace('_', '-'))
-    if certificate and no_ssl_verify:
+    if ca_certificate and no_ssl_verify:
         raise click.BadArgumentUsage(
-            'Option "--no-ssl-verify" and option "--certificate" are mutually exclusive.')
+            'Option "--no-ssl-verify" and option "--ca-certificate" are mutually exclusive.')
 
     ssl_verify = True
-    if certificate:
-        ssl_verify = certificate
+    if ca_certificate:
+        ssl_verify = ca_certificate
     if no_ssl_verify:
         ssl_verify = False
 
@@ -215,10 +215,6 @@ def main(ctx, username, password,
         scheme=scheme, apiUrl=api_url, apiKey=api_key, sslVerify=ssl_verify, token=token,
         retries=retries)
 
-    if certificate and ctx.obj.scheme != 'https':
-        raise click.BadArgumentUsage(
-            'A URI scheme of "https" is required for option "--certificate"')
-
 
 def _set_logging_level(verbosity):
     if not verbosity:
Repository: girder/girder
Issue: CherryPy 12 removed Response timeout parameter

CherryPy Response objects used to have a `timeout` parameter that defaulted to 300 seconds.  We occasionally needed more time that this for responses, so would reset the timer in the `utility/progress/setResponseTimeLimit` function.  As of CherryPy 12.0, the Response objects no longer have this parameter (see https://github.com/cherrypy/cherrypy/issues/1625).

Since we only ever use this to extend the timeout, at least in core code, we could make this a no-op if the Response object doesn't have a timeout property.  Further, to warn external code that uses this function, if called with non-default parameters, we could emit a warning that this feature is no longer available through CherryPy.

Changed files:
girder/utility/progress.py

diff --git a/girder/utility/progress.py b/girder/utility/progress.py
index 681b8c043..56b98e73c 100644
--- a/girder/utility/progress.py
+++ b/girder/utility/progress.py
@@ -108,19 +108,25 @@ noProgress = ProgressContext(False)
 def setResponseTimeLimit(duration=600, onlyExtend=True):
     """
     If we are currently within a cherrypy response, extend the time limit.  By
-    default, cherrypy responses will timeout after 300 seconds, so any activity
-    which can take longer should call this function.
+    default, cherrypy (version < 12.0) responses will timeout after 300
+    seconds, so any activity which can take longer should call this function.
 
     Note that for cherrypy responses that include streaming generator
     functions, such as downloads, the timeout is only relevant until the first
     ``yield`` is reached.  As such, long running generator responses do not
     generally need to call this function.
 
+    @deprecated - remove this function once we pin to CherryPy >= 12.0
+
     :param duration: additional duration in seconds to allow for the response.
     :param onlyExtend: if True, only ever increase the timeout.  If False, the
                        new duration always replaces the old one.
     """
-    if cherrypy.response and getattr(cherrypy.response, 'time'):
+    # CherryPy 12.0 no longer has a timeout propery on a response.  Since we
+    # had only been using this to extend the time, if the timeout property is
+    # not present, do nothing.
+    if (cherrypy.response and getattr(cherrypy.response, 'time', None) and
+            getattr(cherrypy.response, 'timeout', None)):
         newTimeout = time.time() - cherrypy.response.time + duration
         if not onlyExtend or newTimeout > cherrypy.response.timeout:
             cherrypy.response.timeout = newTimeout
Repository: comictagger/comictagger
Issue: Advertisment vs Advertisement

The schema: https://github.com/anansi-project/comicinfo/blob/main/schema/v2.0/ComicInfo.xsd
Has the page type as: "Advertisment" (no e)
Our code has: "Advertisement" (with e)

I noticed the issue when loading an archive with the schema's value (no e), and the application crashes on save (as below).

It is a simple change and I can create a pull request - but then all old archives with the old spelling could similarly crash.

A cleaner approach might be to default the old value to the new one silently, but it would require an archive save to fix permanently in the archive's xml file.

Thoughts?


Traceback (most recent call last):
  File "/home/michael.fitzurka/Projects/comictagger/comictaggerlib/taggerwindow.py", line 2063, in fileListSelectionChanged
    self.loadArchive(fi.ca)
  File "/home/michael.fitzurka/Projects/comictagger/comictaggerlib/taggerwindow.py", line 2081, in loadArchive
    self.actualLoadCurrentArchive()
  File "/home/michael.fitzurka/Projects/comictagger/comictaggerlib/taggerwindow.py", line 610, in actualLoadCurrentArchive
    self.pageListEditor.setData(self.comic_archive, self.metadata.pages)
  File "/home/michael.fitzurka/Projects/comictagger/comictaggerlib/pagelisteditor.py", line 275, in setData
    item = QListWidgetItem(self.listEntryText(p))
  File "/home/michael.fitzurka/Projects/comictagger/comictaggerlib/pagelisteditor.py", line 287, in listEntryText
    text += " (" + self.pageTypeNames[page_dict['Type']] + ")"
KeyError: 'Advertisment'
Traceback (most recent call last):
  File "/home/michael.fitzurka/Projects/comictagger/comictaggerlib/taggerwindow.py", line 2063, in fileListSelectionChanged
    self.loadArchive(fi.ca)
  File "/home/michael.fitzurka/Projects/comictagger/comictaggerlib/taggerwindow.py", line 2067, in loadArchive
    self.clearForm()
  File "/home/michael.fitzurka/Projects/comictagger/comictaggerlib/taggerwindow.py", line 748, in clearForm
    self.pageListEditor.setData(self.comic_archive, self.metadata.pages)
  File "/home/michael.fitzurka/Projects/comictagger/comictaggerlib/pagelisteditor.py", line 271, in setData
    self.listWidget.itemSelectionChanged.disconnect(self.changePage)
TypeError: 'method' object is not connected
Aborted (core dumped)


Changed files:
comictaggerlib/pagelisteditor.py

diff --git a/comictaggerlib/pagelisteditor.py b/comictaggerlib/pagelisteditor.py
index 43aa817..8c40bb7 100644
--- a/comictaggerlib/pagelisteditor.py
+++ b/comictaggerlib/pagelisteditor.py
@@ -284,7 +284,10 @@ class PageListEditor(QWidget):
     def listEntryText(self, page_dict):
         text = str(int(page_dict['Image']) + 1)
         if 'Type' in page_dict:
-            text += " (" + self.pageTypeNames[page_dict['Type']] + ")"
+            if page_dict["Type"] in self.pageTypeNames.keys():
+                text += " (" + self.pageTypeNames[page_dict['Type']] + ")"
+            else:
+                text += " (Error: " + page_dict["Type"] + ")"
         return text
 
     def getPageList(self):