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

trying to add logging for OCaml RPC calls #10196

Closed
wants to merge 1 commit into from
Closed

Conversation

aryx
Copy link
Collaborator

@aryx aryx commented May 2, 2024

test plan:

$ SEMGREP_LOG_SRCS=.* semgrep --test go/lang/correctness/permissions/ --debug
...
calling OCaml via rpc

but I don't see any logs from the OCaml side :(

test plan:
```
$ SEMGREP_LOG_SRCS=.* semgrep --test go/lang/correctness/permissions/ --debug
...
calling OCaml via rpc
```

but I don't see any logs from the OCaml side :(
@aryx aryx requested review from a team, IagoAbal and emjin and removed request for a team May 2, 2024 13:57
Copy link
Contributor

github-actions bot commented May 2, 2024

PR checklist:

  • Purpose of the code is evident to future readers
  • Tests included or PR comment includes a reproducible test plan
  • Documentation is up-to-date
  • A changelog entry was added to changelog.d for any user-facing change
  • Change has no security implications (otherwise, ping security team)

If you're unsure about any of this, please see:

@aryx aryx requested review from amchiclet and nmote and removed request for IagoAbal and emjin May 2, 2024 13:58
@aryx
Copy link
Collaborator Author

aryx commented May 2, 2024

if I put in ocaml.py to the call to subprocess.Popen(..., stderr=sys.stderr),
then the RPC autofix does not seem to work @nmote .
Any idea how we can enable logging for the ocaml side of the RPC so we can use Logs.xxx or Log.xxx
in src/rpc/?

Copy link
Contributor

github-actions bot commented May 2, 2024

📸 The pytest shapshots changed in your PR.
Please carefully review these changes and make sure they are intended:

  1. Review the changes at c8c0325

  2. Accept the new snapshots with

    git fetch origin && git cherry-pick c8c0325 && git push
    

@nmote
Copy link
Collaborator

nmote commented May 2, 2024

I would have thought that setting stderr the way you describe would have done the trick... I will try to take a stab at this next week if you don't figure it out before then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants