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

minor fix for stablility #5

Closed

Conversation

LittleLittleCloud
Copy link
Collaborator

Why are these changes needed?

Related issue number

Checks

@codecov-commenter
Copy link

codecov-commenter commented Sep 21, 2023

Codecov Report

Merging #5 (5d727dd) into main (3627ca4) will increase coverage by 0.17%.
Report is 2 commits behind head on main.
The diff coverage is 55.55%.

@@            Coverage Diff             @@
##             main       #5      +/-   ##
==========================================
+ Coverage   34.51%   34.68%   +0.17%     
==========================================
  Files          17       17              
  Lines        1921     1923       +2     
  Branches      420      419       -1     
==========================================
+ Hits          663      667       +4     
  Misses       1207     1207              
+ Partials       51       49       -2     
Flag Coverage Δ
unittests 34.63% <55.55%> (+0.17%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
autogen/agentchat/conversable_agent.py 64.95% <33.33%> (+0.93%) ⬆️
autogen/code_utils.py 45.62% <66.66%> (+0.08%) ⬆️

@sonichi
Copy link
Contributor

sonichi commented Sep 24, 2023

To run the OpenAI test, we need the PR to be made from a branch in the original repo. Could you make the PR that way? Or if you know an alternative that allows running the test even from forked repo, please suggest.

@LittleLittleCloud
Copy link
Collaborator Author

To run the OpenAI test, we need the PR to be made from a branch in the original repo. Could you make the PR that way? Or if you know an alternative that allows running the test even from forked repo, please suggest.

To use the context from the base of repo to fork repo when running an action or workflow, you can use pull_request_target as trigger. Make sure you add safety check (like additional approval before running OpenAI job) because running action from untrusted source might cause secret leak. You can find more information from this blog

@sonichi
Copy link
Contributor

sonichi commented Sep 25, 2023

To run the OpenAI test, we need the PR to be made from a branch in the original repo. Could you make the PR that way? Or if you know an alternative that allows running the test even from forked repo, please suggest.

To use the context from the base of repo to fork repo when running an action or workflow, you can use pull_request_target as trigger. Make sure you add safety check (like additional approval before running OpenAI job) because running action from untrusted source might cause secret leak. You can find more information from this blog

I've tried this before but it didn't work. Feel free to give it another try if you want.

@LittleLittleCloud
Copy link
Collaborator Author

To run the OpenAI test, we need the PR to be made from a branch in the original repo. Could you make the PR that way? Or if you know an alternative that allows running the test even from forked repo, please suggest.

To use the context from the base of repo to fork repo when running an action or workflow, you can use pull_request_target as trigger. Make sure you add safety check (like additional approval before running OpenAI job) because running action from untrusted source might cause secret leak. You can find more information from this blog

I've tried this before but it didn't work. Feel free to give it another try if you want.

I can take a look, I'll need permission to push to original repo though, can you grant me that (contributor or admin should both work)

@sonichi
Copy link
Contributor

sonichi commented Sep 26, 2023

To run the OpenAI test, we need the PR to be made from a branch in the original repo. Could you make the PR that way? Or if you know an alternative that allows running the test even from forked repo, please suggest.

To use the context from the base of repo to fork repo when running an action or workflow, you can use pull_request_target as trigger. Make sure you add safety check (like additional approval before running OpenAI job) because running action from untrusted source might cause secret leak. You can find more information from this blog

I've tried this before but it didn't work. Feel free to give it another try if you want.

I can take a look, I'll need permission to push to original repo though, can you grant me that (contributor or admin should both work)

you should have maintainer permission already.

@LittleLittleCloud
Copy link
Collaborator Author

To run the OpenAI test, we need the PR to be made from a branch in the original repo. Could you make the PR that way? Or if you know an alternative that allows running the test even from forked repo, please suggest.

To use the context from the base of repo to fork repo when running an action or workflow, you can use pull_request_target as trigger. Make sure you add safety check (like additional approval before running OpenAI job) because running action from untrusted source might cause secret leak. You can find more information from this blog

I've tried this before but it didn't work. Feel free to give it another try if you want.

I can take a look, I'll need permission to push to original repo though, can you grant me that (contributor or admin should both work)

you should have maintainer permission already.

Did you enable Required reviewers on OpenAI environemnt? Can't see that environment page on my end
image

@sonichi
Copy link
Contributor

sonichi commented Sep 26, 2023

To run the OpenAI test, we need the PR to be made from a branch in the original repo. Could you make the PR that way? Or if you know an alternative that allows running the test even from forked repo, please suggest.

To use the context from the base of repo to fork repo when running an action or workflow, you can use pull_request_target as trigger. Make sure you add safety check (like additional approval before running OpenAI job) because running action from untrusted source might cause secret leak. You can find more information from this blog

I've tried this before but it didn't work. Feel free to give it another try if you want.

I can take a look, I'll need permission to push to original repo though, can you grant me that (contributor or admin should both work)

you should have maintainer permission already.

Did you enable Required reviewers on OpenAI environemnt? Can't see that environment page on my end image

Yes I did.

@LittleLittleCloud
Copy link
Collaborator Author

Cool, then you can go check #21 out. The secureness of secrets should be able to be protected by Environemnt Protection Rule.

Note that this still brings side effects, as the external source will still be run on target environment once it gets approval, so the secureness will rely on reviewers, who might make mistakes. It's the cost we have to pay if we want a contribution from the forked repo though.

In order to add another layer of secure, you might want to enable Require approval from all outside collaborator as well
image

@LittleLittleCloud
Copy link
Collaborator Author

I'm going to create another PR from main repo to avoid blocking by #21

@LittleLittleCloud LittleLittleCloud mentioned this pull request Sep 28, 2023
3 tasks
randombet pushed a commit to randombet/autogen that referenced this pull request Sep 9, 2024
* Fix Anthropic system messages

* Update anthropic.py update separator for system messages
jackgerrits pushed a commit that referenced this pull request Oct 2, 2024
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