Skip to content

Conversation

@wqyenjoy
Copy link
Contributor

feat: format imports and finalize MetadataServiceV2 implementation

  • Format import statements using imports-formatter with three distinct blocks
  • Ensure proper separation of standard library, third-party, and internal packages
  • Finalize MetadataServiceV2 implementation for Java 3.3.1 compatibility
  • Add comprehensive unit tests for MetadataServiceV2 functionality
  • Implement dual protocol architecture supporting both V1 and V2 versions

@AlexStocks AlexStocks requested review from FoghostCn, Copilot and marsevilspirit and removed request for Copilot and marsevilspirit August 24, 2025 09:59
@wqyenjoy wqyenjoy force-pushed the feature/metadata-service-tri-export-clean branch from 1fda49d to d120d00 Compare August 24, 2025 10:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

// exportV2Services exports V2 metadata service on same port with different interface
// This ensures clients can discover V2 service using standard discovery mechanisms
func (e *serviceExporter) exportV2Services(port string) error {
return e.exportV2(port)
Copy link
Contributor

Choose a reason for hiding this comment

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

这种一行代码的函数就没必要了吧?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是的

// do not set, because it will override MetadataService
//exporter.metadataService.SetMetadataServiceURL(ivkURL)

// Both V1 and V2 are now discoverable on same port with different interface names
Copy link
Contributor

Choose a reason for hiding this comment

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

e.opts.protocol == 'tri or 'dubbo' not both

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

Comment on lines +361 to +363
if metadataInfo == nil {
return nil, nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Silent failure is not good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i will return a clear failure

@wqyenjoy wqyenjoy force-pushed the feature/metadata-service-tri-export-clean branch 6 times, most recently from 0192a8b to 9e2f73b Compare August 30, 2025 03:53
…lity

- Add MetadataServiceV2 implementation with triple protocol support
- Fix import formatting issues in metadata test files
- Update import block ordering for imports-formatter compatibility
- Add .cursorrules to .gitignore for local development rules
- Improve metadata service error handling and compatibility
@wqyenjoy wqyenjoy force-pushed the feature/metadata-service-tri-export-clean branch from 92d2e1f to c9d7e7d Compare August 30, 2025 04:21
john added 3 commits August 30, 2025 12:37
- Update import indentation to match imports-formatter tool
- Ensure CI formatting checks pass without modifications
- Remove unused serviceInvoker type and Invoke method
- Remove unused convertV1 function
- Remove unused emptyStrSlice variable in zookeeper report
- Remove unused result package import

Fixes golangci-lint errors for unused code
- Clean up empty lines in import sections
- Ensure consistent import block formatting
- Maintain imports-formatter compliance
@codecov-commenter
Copy link

codecov-commenter commented Aug 30, 2025

Codecov Report

❌ Patch coverage is 61.40351% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.52%. Comparing base (97a472c) to head (d239049).

Files with missing lines Patch % Lines
metadata/metadata_service.go 61.40% 19 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2983      +/-   ##
===========================================
+ Coverage    39.36%   39.52%   +0.16%     
===========================================
  Files          457      457              
  Lines        38863    38891      +28     
===========================================
+ Hits         15299    15373      +74     
+ Misses       22304    22254      -50     
- Partials      1260     1264       +4     

☔ 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.

john added 3 commits August 30, 2025 14:15
- Remove unnecessary single-line functions (shouldExportV2, exportV2Services)
- Remove unused service discovery registration for metadata service
- Fix silent failure issues by returning proper error messages
- Update test cases to handle new error return behavior
- Remove duplicate import declaration

Addresses reviewer feedback:
- AlexStocks: Simplified single-line functions
- FoghostCn: Removed unnecessary service discovery, fixed silent failures
- Reorganize imports into standard three-block format:
  * Block 1: Standard library imports
  * Block 2: Third-party library imports
  * Block 3: Internal project imports
- Ensure consistent import ordering and formatting
- Maintain compliance with project's import style guide
requestData = make(map[string]any)
}

log.Printf("请求数据: %+v", requestData)

Check notice

Code scanning / SonarCloud

Logging should not be vulnerable to injection attacks Low

Change this code to not log user-controlled data. See more on SonarQube Cloud
return
}

log.Printf("转换参数: types=%v, args=%+v", methodSchema.ParameterTypes, tripleArgs)

Check notice

Code scanning / SonarCloud

Logging should not be vulnerable to injection attacks Low

Change this code to not log user-controlled data. See more on SonarQube Cloud

// 写入错误响应
func (gw *TripleGateway) writeErrorResponse(w http.ResponseWriter, statusCode int, message string) {
log.Printf("错误响应: %d - %s", statusCode, message)

Check notice

Code scanning / SonarCloud

Logging should not be vulnerable to injection attacks Low

Change this code to not log user-controlled data. See more on SonarQube Cloud
@wqyenjoy wqyenjoy force-pushed the feature/metadata-service-tri-export-clean branch from 8c41fdb to 5aba962 Compare September 13, 2025 00:34
@sonarqubecloud
Copy link

@Alanxtl
Copy link
Contributor

Alanxtl commented Oct 26, 2025

pls fix ci fail

@Alanxtl Alanxtl added the pending PR Don't merge label Nov 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants