Skip to content

Conversation

@katara-Jayprakash
Copy link
Member

@katara-Jayprakash katara-Jayprakash commented Jan 7, 2026

introduce python linting in agent code
fix's the part of:#81

@gemini-code-assist
Copy link
Contributor

Note

Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported.

@acsoto
Copy link
Contributor

acsoto commented Jan 7, 2026

#95

@codecov-commenter
Copy link

codecov-commenter commented Jan 7, 2026

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@f0cebc3). Learn more about missing BASE report.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #121   +/-   ##
=======================================
  Coverage        ?   29.20%           
=======================================
  Files           ?       28           
  Lines           ?     2551           
  Branches        ?        0           
=======================================
  Hits            ?      745           
  Misses          ?     1690           
  Partials        ?      116           
Flag Coverage Δ
unittests 29.20% <ø> (?)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@katara-Jayprakash
Copy link
Member Author

katara-Jayprakash commented Jan 7, 2026

#95

ouch! sorry sir , but you did not assign the epic so I thought it was free to work, or may be it was me who did not see the other open issue

@acsoto
Copy link
Contributor

acsoto commented Jan 7, 2026

#95

ouch! sorry sir , but you did not assign the epic so I thought it was free to work, or may be it was me who did not see the other open issue

It's all good! I just wanted to point it out so we don't have duplicate efforts. Thanks for your contribution

Copy link
Member

@hzxuzhonghu hzxuzhonghu left a comment

Choose a reason for hiding this comment

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

@LiZhenCheng9527 we should use ruff, which seems more widely used

pull_request:
branches:
- main
- release
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- release
- release-*

Copy link
Member Author

Choose a reason for hiding this comment

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

added this in next commit, i should be more focused

- "**.py"
- "cmd/cli/pyproject.toml"
- "sdk-python/pyproject.toml"
push:
Copy link
Member

Choose a reason for hiding this comment

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

no need add this trigger, this means it is merged

Copy link
Member Author

Choose a reason for hiding this comment

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

ya thats actually make sense, we did not need to check the linting after pr merged.

python-version: "3.10"
- name: Run Lint
run: |
make lint-python
Copy link
Member

Choose a reason for hiding this comment

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

I can not find this command in Makefile

Copy link
Member Author

Choose a reason for hiding this comment

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

ya i try to write it myself but fails, so shift to writing into the workflow

Makefile Outdated
lint: golangci-lint ## Run golangci-lint
$(GOLANGCI_LINT) run ./...

.PHONY: lint-python
Copy link
Member Author

Choose a reason for hiding this comment

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

why this doesn't work here? just curious it fail with gen-check all

@hzxuzhonghu
Copy link
Member

@acsoto Please take a look at the python change

@hzxuzhonghu hzxuzhonghu requested a review from Copilot January 14, 2026 02:50

# Sandbox / CodeInterpreter configuration
CODEINTERPRETER_NAME = os.environ.get("CODEINTERPRETER_NAME", "simple-codeinterpreter")
SANDBOX_NAMESPACE = os.environ.get("SANDBOX_NAMESPACE", "default")
Copy link
Member

Choose a reason for hiding this comment

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

why not allow customize namespace?

@volcano-sh-bot
Copy link
Contributor

Adding label do-not-merge/contains-merge-commits because PR contains merge commits, which are not allowed in this repository.
Use git rebase to reapply your commits on top of the target branch. Detailed instructions for doing so can be found here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

- Add GitHub workflow for Python linting using Ruff
- Fix import ordering and formatting issues

Signed-off-by: katara-Jayprakash <katarajayprakash@icloud.com>
@katara-Jayprakash
Copy link
Member Author

/cc @acsoto @hzxuzhonghu finally done! review and merge it sir!

@acsoto
Copy link
Contributor

acsoto commented Jan 19, 2026

/lgtm

@volcano-sh-bot
Copy link
Contributor

@acsoto: changing LGTM is restricted to collaborators

Details

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@katara-Jayprakash
Copy link
Member Author

/lgtm

thanx so we can merge it now?

@hzxuzhonghu
Copy link
Member

/lgtm

@hzxuzhonghu
Copy link
Member

/hold cancel

@volcano-sh-bot volcano-sh-bot merged commit 7637e5f into volcano-sh:main Jan 19, 2026
12 checks passed
@katara-Jayprakash
Copy link
Member Author

thanx sir,✨

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants