Skip to content

UTAPI-116: Check stderr and fix deprecation warnings #1318

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.

Sign up for GitHub

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

Jump to bottom
Merged
bert-e merged 6 commits into from
Aug 28, 2025
Merged

UTAPI-116: Check stderr and fix deprecation warnings #1318

bert-e merged 6 commits into from
Aug 28, 2025

Conversation

@BourgoisMickael
Copy link
Contributor

@BourgoisMickael BourgoisMickael commented Aug 27, 2025 •
edited
Loading

It floods the stderr logs of cloudserver and utapi

(node:4634) [DEP0174] DeprecationWarning: Calling promisify on a function that returns a Promise is likely a mistake.

@bert-e
Copy link
Contributor

bert-e commented Aug 27, 2025

Hello bourgoismickael,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options
name description privileged authored
/after_pull_request Wait for the given pull request id to be merged before continuing with the current one.
/bypass_author_approval Bypass the pull request author's approval ⭐
/bypass_build_status Bypass the build and test status ⭐
/bypass_commit_size Bypass the check on the size of the changeset TBA ⭐
/bypass_incompatible_branch Bypass the check on the source branch prefix ⭐
/bypass_jira_check Bypass the Jira issue check ⭐
/bypass_peer_approval Bypass the pull request peers' approval ⭐
/bypass_leader_approval Bypass the pull request leaders' approval ⭐
/approve Instruct Bert-E that the author has approved the pull request. ✍️
/create_pull_requests Allow the creation of integration pull requests.
/create_integration_branches Allow the creation of integration branches.
/no_octopus Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
/unanimity Change review acceptance criteria from one reviewer at least to all reviewers
/wait Instruct Bert-E not to run until further notice.
Available commands
name description privileged
/help Print Bert-E's manual in the pull request.
/status Print Bert-E's current status in the pull request TBA
/clear Remove all comments from Bert-E from the history TBA
/retry Re-start a fresh build TBA
/build Re-start a fresh build TBA
/force_reset Delete integration branches & pull requests, and restart merge process from the beginning.
/reset Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

@bert-e
Copy link
Contributor

bert-e commented Aug 27, 2025

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • 2 peers

@BourgoisMickael BourgoisMickael force-pushed the bugfix/UTAPI-116-stderr-warnings branch 8 times, most recently from 4de62a5 to 38badb7 Compare August 27, 2025 21:25
@BourgoisMickael BourgoisMickael requested a review from a team August 27, 2025 21:37
tcarmet
tcarmet approved these changes Aug 27, 2025
View reviewed changes
@BourgoisMickael BourgoisMickael requested review from a team, jonathan-gramain, tcarmet and tmacro and removed request for tcarmet August 27, 2025 22:59
jonathan-gramain
jonathan-gramain reviewed Aug 27, 2025
View reviewed changes
.github/workflows/tests.yaml
env: ${{ matrix.test.env }}
- name: Print stderr logs
run: grep -H "" *.stderr.log || true
- name: Error if stderr logs are not empty
Copy link
Contributor

@jonathan-gramain jonathan-gramain Aug 27, 2025

Choose a reason for hiding this comment

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

Maybe a bit extreme to fail the test? IDK, just a personal feeling, it's useful to have feedback on any new stderr message, maybe we can add a continue-on-error: true so it's visible but doesn't outright fail the test if the overall test suite is successful.

Copy link
Contributor Author

@BourgoisMickael BourgoisMickael Aug 27, 2025

Choose a reason for hiding this comment

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

That's the idea to catch immediately a warning, so we are not surprised later.

Then if we don't plan on fixing the warning, we can filter it out from the logs (38badb7)

A continue-on-error: true could be added at any moment if there are warnings that can't be fixed to pass the tests, but ideally I'd like to fail to make sure we notice it.

And I'd like to have this in other components as well in long term

libV2/redis.js Outdated
* @param {Function} originalFn The function to promisify.
* @returns {Function} A function that returns a promise.
*/
function flexiblePromisify(originalFn) {
Copy link
Contributor

@jonathan-gramain jonathan-gramain Aug 27, 2025

Choose a reason for hiding this comment

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

Is this a standard, proven function?

  • if so, can you provide the ref
  • if not, can you add unit tests to make sure it works as designed?

Copy link
Contributor Author

@BourgoisMickael BourgoisMickael Aug 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Nothing standard, made with gemini

here are unit tests: 7a12ad2

@BourgoisMickael
UTAPI-116: Suppress aws sdk v2 warnings
b199d52
@BourgoisMickael
UTAPI-116: Collect stderr and check for logs
debad24
@BourgoisMickael
UTAPI-116: Fix node22 warning DEP0174 promisify
b7b43de 
It floods the stderr logs of cloudserver and utapi

```
(node:4634) [DEP0174] DeprecationWarning: Calling promisify on a function that returns a Promise is likely a mistake.
```
@BourgoisMickael
UTAPI-116: Fix warning MaxListenersExceededWarning
cada2a6 
In the cleanup of functional/v2/task/testCreateSnapshot.js
@BourgoisMickael
UTAPI-116: Ignore DEP0040 punycode from oas-tools
2fb5e55
@BourgoisMickael
UTAPI-116: Unit tests for flexiblePromisify
71df842
@BourgoisMickael BourgoisMickael force-pushed the bugfix/UTAPI-116-stderr-warnings branch from 7a12ad2 to 71df842 Compare August 28, 2025 16:46
@BourgoisMickael
Copy link
Contributor Author

BourgoisMickael commented Aug 28, 2025

/approve

@bert-e
Copy link
Contributor

bert-e commented Aug 28, 2025

I have successfully merged the changeset of this pull request
into targetted development branches:

  • ✔️ development/8.2

The following branches have NOT changed:

  • development/6.4
  • development/7.10
  • development/7.4
  • development/7.70
  • development/8.1

Please check the status of the associated issue UTAPI-116.

Goodbye bourgoismickael.

The following options are set: approve

@bert-e bert-e merged commit 71df842 into development/8.2 Aug 28, 2025
15 checks passed
@bert-e bert-e deleted the bugfix/UTAPI-116-stderr-warnings branch August 28, 2025 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jonathan-gramain jonathan-gramain jonathan-gramain approved these changes

@tcarmet tcarmet tcarmet approved these changes

@tmacro tmacro tmacro approved these changes

@anurag4DSB anurag4DSB Awaiting requested review from anurag4DSB

@dvasilas dvasilas Awaiting requested review from dvasilas

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@BourgoisMickael @bert-e @jonathan-gramain @tcarmet @tmacro