GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-106217: Truncate the issue body size of `new-bugs-announce-notifier` by sobolevn · Pull Request #106329 · python/cpython

Conversation

@sobolevn

@sobolevn sobolevn commented Jul 2, 2023 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: new-bugs-announce notifier workflow fails on large bug reports #106217

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@sobolevn
pythongh-106217: Trunkate the issue body size of `new-bugs-announce-n…
cd71efa 
…otifier`
@sobolevn
sobolevn requested review from ezio-melotti and hugovk as code owners July 2, 2023 07:22
@bedevere-bot bedevere-bot added the awaiting review label Jul 2, 2023
@bedevere-bot bedevere-bot mentioned this pull request Jul 2, 2023
new-bugs-announce notifier workflow fails on large bug reports #106217
Closed
@sobolevn sobolevn added the skip news label Jul 2, 2023
hugovk
hugovk approved these changes Jul 2, 2023
View reviewed changes

@hugovk hugovk left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

See also python/core-workflow#456.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread .github/workflows/new-bugs-announce-notifier.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Jul 2, 2023
@sobolevn @hugovk
Update .github/workflows/new-bugs-announce-notifier.yml
597b4dd 
Co-authored-by: Hugo van Kemenade <[email protected]>
@hugovk
hugovk merged commit 5950e7d into python:main Jul 2, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Jul 2, 2023
@hugovk hugovk mentioned this pull request Jul 2, 2023
CPython notify-new-bugs-announce workflow fails for long issue body python/core-workflow#456
Closed
carljm added a commit to carljm/cpython that referenced this pull request Jul 3, 2023
@carljm
Merge branch 'main' into modulelocals
ac11740 
* main: (167 commits)
  pythongh-91053: make func watcher tests resilient to other func watchers (python#106286)
  pythongh-104050: Add more type hints to Argument Clinic DSLParser() (python#106354)
  pythongh-106359: Fix corner case bugs in Argument Clinic converter parser (python#106361)
  pythongh-104146: Remove unused attr 'parameter_indent' from clinic.DLParser (python#106358)
  pythongh-106320: Remove private _PyErr C API functions (python#106356)
  pythongh-104050: Annotate Argument Clinic DSLParser attributes (python#106357)
  pythongh-106320: Create pycore_modsupport.h header file (python#106355)
  pythongh-106320: Move _PyUnicodeWriter to the internal C API (python#106342)
  pythongh-61215: New mock to wait for multi-threaded events to happen (python#16094)
  Document PYTHONSAFEPATH along side -P (python#106122)
  Replace the esoteric term 'datum' when describing dict comprehensions (python#106119)
  pythongh-104050: Add more type hints to Argument Clinic DSLParser() (python#106343)
  pythongh-106320: _testcapi avoids private _PyUnicode_EqualToASCIIString() (python#106341)
  pythongh-106320: Add pycore_complexobject.h header file (python#106339)
  pythongh-106078: Move DecimalException to _decimal state (python#106301)
  pythongh-106320: Use _PyInterpreterState_GET() (python#106336)
  pythongh-106320: Remove private _PyInterpreterState functions (python#106335)
  pythongh-104922: Doc: add note about PY_SSIZE_T_CLEAN (python#106314)
  pythongh-106217: Truncate the issue body size of `new-bugs-announce-notifier` (python#106329)
  pythongh-104922: remove PY_SSIZE_T_CLEAN (python#106315)
  ...
@jkloth

jkloth commented Jul 4, 2023 •
edited by hugovk
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

It seems that now there is no body for any message sent via the notifier after this change. See the new-bugs-announce archives for issue 106333 (has body) vs 106334 (no body)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hugovk

hugovk commented Jul 4, 2023

Copy link
Copy Markdown
Member

Aha, looking at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/substring, the args are:

substring(indexStart)
substring(indexStart, indexEnd)

indexStart
The index of the first character to include in the returned substring.

indexEnd Optional
The index of the first character to exclude from the returned substring.

So:

issue.data.body.substring(8000)

is attempting to get the substring from char number 8000 up to the end, meaning we lose the first 8000 chars, and get an empty string if there's nothing beyond that. For example:

const str = 'Mozilla';
console.log(str.substring(4));
// prints "lla"
console.log(str.substring(8000));
// prints ""

Instead we want something like issue.data.body.substring(0, 8000) for the substring from 0 up to 8000.

console.log(str.substring(0, 4));
// "Mozi"
console.log(str.substring(0, 8000));
// "Mozilla"

Does that look right? Would you like to make a new PR?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sobolevn

sobolevn commented Jul 4, 2023

Copy link
Copy Markdown
Member Author

Oh my, js strikes again 🤦
hugovk reacted with laugh emoji
All reactions
  • 😄 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sobolevn sobolevn mentioned this pull request Jul 4, 2023
gh-106217: Truncate the issue body size of new-bugs-announce-notifier #106423
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hugovk hugovk hugovk approved these changes
@ezio-melotti ezio-melotti Awaiting requested review from ezio-melotti ezio-melotti is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@sobolevn @jkloth @hugovk @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.