Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config_parser: further futurize #130

Merged
merged 2 commits into from
Nov 29, 2019
Merged

Conversation

joshmoore
Copy link
Member

Necessary for work on the documentation merge build:
ome/omero-documentation#2035

Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

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

With this PR, bin/omero config parse --rst now goes beyond the initial sort error but fails with some encoding error.

In my local test, this was caused by the Unicode characters in https://github.com/ome/omero-web/blob/v5.6.dev6/omeroweb/settings.py#L591. As discussed with @joshmoore, we might want to have some unit test in place either here and/or in omero-web covering the encoding use case.

@@ -372,8 +372,8 @@ def headers(self):
additional_headers[section] = Header(section)
headers.setdefault(additional_headers[section], []).append(x)

for key in headers.keys():
headers[key].sort(lambda a, b: cmp(a.key, b.key))
for key in list(headers.keys()):
Copy link
Member

Choose a reason for hiding this comment

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

Don't need list here really?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm.... pretty sure this is coming from the call to futurize -w -0, i.e. being overly cautious. Is it worth pushing away?

Copy link
Member

Choose a reason for hiding this comment

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

Probably not worth the trouble.

@joshmoore joshmoore merged commit 645fb31 into ome:master Nov 29, 2019
@joshmoore joshmoore deleted the omero-install branch November 29, 2019 11:34
@joshmoore joshmoore added this to the 5.6.0 milestone Mar 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants