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

Fix TypeError for fsoids #351

Merged
merged 2 commits into from Oct 28, 2021
Merged

Fix TypeError for fsoids #351

merged 2 commits into from Oct 28, 2021

Conversation

ale-rt
Copy link
Member

@ale-rt ale-rt commented Aug 25, 2021

Fix TypeError: can't concat str to bytes when running fsoids.py script with Python 3.

Closes #350

Fix ``TypeError: can't concat str to bytes`` when running fsoids.py script with Python 3.

Closes #350
@ale-rt
Copy link
Member Author

ale-rt commented Aug 25, 2021

I made the patch in such a way that the shorten function can handle bytes as well as text.
The code we have now on master actually says we should deal with strings:

# Shorten a string for display.
def shorten(s, size=50):
if len(s) <= size:
return s
# Stick ... in the middle.
navail = size - 5
nleading = navail // 2
ntrailing = size - nleading
return s[:nleading] + " ... " + s[-ntrailing:]

One other possibility is to enforce the shorten function to refuse any output that is not a native string or to convert it to a native string, but this appears to me that increases the change of breaking some other stuff...

Suggestions are welcome.

Once it is clear what to do I can even add some more tests.

@@ -5,6 +5,9 @@
5.6.1 (unreleased)
==================

- Fix ``TypeError: can't concat str to bytes`` when running fsoids.py script with Python 3.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok

@jmuchemb
Copy link
Member

It seems that we could simply write:

     return s[:nleading] + b" ... " + s[-ntrailing:]

because if I read the code correctly, s is always of type bytes. To be checked with a pdb.

@ale-rt
Copy link
Member Author

ale-rt commented Aug 27, 2021

@jmuchemb I also never found that the function argument is text.

Anyway the comment says # Shorten a string for display.
While that is correct in Python 2, on Python 3 it is not true.

With this patch, the output of this script is something like:

oid 0x46b8 ZODB.blob.Blob 2 revisions
    tid 0x03e1ec746d9d5a77 offset=32500086 2021-08-06 13:40:25.690936
        tid user=b' ale'
        tid description=b'/Plone/workspaces/...'

Note the b'ale' and the b'/Plone/workspaces/...'.

I think that this is an error that should be fixed as well, there is no reason print the str representation of those bytes instances.

In short this patch does not solve the misuse of bytes and text, it just wants to make the script work again.

@icemac
Copy link
Member

icemac commented Oct 26, 2021

Is this PR ready to be merged or is there still something to be done to make it ready?

@ale-rt
Copy link
Member Author

ale-rt commented Oct 27, 2021

@icemac I would say this is good to be merged, I had to merge master because of a conflict

@icemac icemac merged commit 1f4c642 into master Oct 28, 2021
@icemac icemac deleted the 350-fix-bytes-vs-text branch October 28, 2021 06:06
@icemac
Copy link
Member

icemac commented Oct 28, 2021

Thank you for this PR and reviewing it. 😃

navytux added a commit to navytux/ZODB that referenced this pull request Oct 29, 2021
to resolve trivial conflict on CHANGES.rst

* origin/master: (22 commits)
  Fix TypeError for fsoids (zopefoundation#351)
  Fix deprecation warnings occurring on Python 3.10.
  fix more PY3 incompatibilities in `fsstats`
  fix Python 3 incompatibility for `fsstats`
  add `fsdump/fsstats` test
  fsdump/fsstats improvements
  - add coverage combine step
  - first cut moving tests from Travis CI to GitHub Actions
  - ignore virtualenv artifacts [ci skip]
  tests: Run race-related tests with high frequency of switches between threads
  tests: Add test for load vs external invalidation race
  tests: Add test for open vs invalidation race
  fixup! doc/requirements: Require pygments < 2.6 on py2
  doc/requirements: Require pygments < 2.6 on py2
  fixup! buildout: Fix Sphinx install on Python2
  buildout: Fix Sphinx install on Python2
  Update README.rst
  Security fix documentation dependencies (zopefoundation#342)
  changes: Correct link to UnboundLocalError fsoids.py fix
  fsrefs: Optimize IO  (take 2) (zopefoundation#340)
  ...
navytux added a commit to navytux/ZODB that referenced this pull request Nov 18, 2021
* y/loadAt.8: (35 commits)
  Make lint happy
  Let the year float.
  Configuring for pure-python
  Specify a PyPy2 version.
  Lint the code.
  Configuring for pure-python
  Fix TypeError for fsoids (zopefoundation#351)
  Fix deprecation warnings occurring on Python 3.10.
  fix more PY3 incompatibilities in `fsstats`
  fix Python 3 incompatibility for `fsstats`
  add `fsdump/fsstats` test
  fsdump/fsstats improvements
  Undeprecate loadBefore
  fixup! changes: Add draft entry for loadAt/loadBeforeEx/DemoStorage fix
  changes: Add draft entry for loadAt/loadBeforeEx/DemoStorage fix
  fixup! Handle NotImplementedError raised by loadBefore/loadBeforeEx as "interface not provided"
  *: Don't emit warnings on loadBefore
  Handle NotImplementedError raised by loadBefore/loadBeforeEx as "interface not provided"
  loadAt -> loadBeforeEx
  - add coverage combine step
  ...
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.

fsoids script fails reporting the shortened tid description on Python 3
4 participants