Skip to content

Conversation

yuokada
Copy link
Contributor

@yuokada yuokada commented Jun 24, 2025

Summary

  • Adds updateExpire method to Java client to support updating table expire days setting
  • Implements the same functionality as td-client-ruby and td-client-python
  • Calls the /v3/table/update API endpoint with expire_days parameter

Changes

  • Added updateExpire(String databaseName, String tableName, int expireDays) method to TDClientApi interface
  • Implemented the method in TDClient class
  • Added comprehensive test coverage including error cases

Test plan

  • Method compiles successfully
  • Added test cases for successful expire days update
  • Added test case for non-existent table error handling
  • Tests follow existing patterns in the codebase

Fixes #110

🤖 Generated with Claude Code

@yuokada yuokada requested a review from Copilot June 24, 2025 08:43
Copy link

@Copilot 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.

Pull Request Overview

This PR adds support for updating the expire days setting for a table by introducing the updateExpire method across the Java client interface and implementation, along with comprehensive tests.

  • Introduces updateExpire in TDClientApi and TDClient
  • Adds tests for both successful updates and an error case for non-existent tables

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/test/java/com/treasuredata/client/TestTDClient.java Added test cases covering updateExpire behavior and error handling
src/main/java/com/treasuredata/client/TDClientApi.java Added updateExpire method signature
src/main/java/com/treasuredata/client/TDClient.java Implemented updateExpire invoking the proper API endpoint
Comments suppressed due to low confidence (1)

src/main/java/com/treasuredata/client/TDClientApi.java:232

  • [nitpick] Consider adding a JavaDoc comment for the updateExpire method to describe its behavior and parameters for consistency with the rest of the API.
    void updateExpire(String databaseName, String tableName, int expireDays);

@xerial
Copy link
Member

xerial commented Jun 24, 2025

Code Review

Thanks for this PR! The implementation looks clean and well-structured. Here are my findings:

✅ Strengths

  • Clean implementation that follows existing patterns in the codebase
  • Proper parameter validation with requireNonNull
  • Comprehensive test coverage including both success and error cases
  • Correctly uses the existing TDUpdateTableResult class
  • Matches the functionality in td-client-ruby and td-client-python

📝 Minor Suggestion

Consider adding JavaDoc documentation to the updateExpire method in TDClientApi.java:232 for consistency with other API methods. Something like:

/**
 * Update the expire days setting for a table
 * 
 * @param databaseName the database name
 * @param tableName the table name
 * @param expireDays the number of days after which data expires
 * @throws TDClientException if the table doesn't exist or other errors occur
 */
void updateExpire(String databaseName, String tableName, int expireDays);

✅ Testing

The test implementation is thorough:

  • Tests successful expire days updates
  • Verifies the change by fetching and checking the table
  • Tests updating to different values
  • Properly handles the non-existent table error case

Overall, this is a solid PR that cleanly adds the requested functionality. LGTM with the minor documentation suggestion above! 👍

yuokada and others added 2 commits August 5, 2025 14:19
- Add updateExpire method to TDClientApi interface
- Implement updateExpire method in TDClient class that calls /v3/table/update API
- Add comprehensive test for updateExpire functionality including error cases
- Method signature matches td-client-ruby and td-client-python implementations

Fixes #110

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@yuokada yuokada force-pushed the feature/support-update-expire-110 branch from ba3f84c to 53c337c Compare August 5, 2025 05:19
@yuokada yuokada enabled auto-merge (squash) August 6, 2025 02:14
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.

Support updateExpire for expire days setting for table

2 participants