Refractored code to remove explicit conversion

[Saurabhdarekar]

Description of changes

As per the change to adapter-framework the requirement to explicitly convert any int64 value to float64 has been resolved. This PR removes the explicit conversion that was done in this repo to remove the ambiguous code.

API References

Pull request intention

• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Code improvement (improve test coverage or code readability)

Pull request checklist

• I have performed a self-review of my own code
• I have added tests that prove my fix is effective or that my feature works
• I have updated documentation (if applicable)
• I have registered the new adapter (if applicable)
• I have added a smoke test for the new adapter (if applicable)
• Any secrets are redacted from smoke test fixtures and no PII is present (if applicable)

[Copilot]

Pull Request Overview

This PR removes explicit int64 to float64 conversions in response to an upstream adapter-framework change that now supports native int64 processing. The change simplifies the codebase by allowing int64 attributes to be parsed directly without conversion to float64.

Key changes:

• Updated parsing logic to handle Int64 and Double attribute types separately
• Modified test expectations to reflect native int64 values instead of float64
• Updated error messages to reflect correct type handling

Reviewed Changes

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

Show a summary per file

File	Description
pkg/my-sql/datasource.go	Separated Int64 and Double parsing with appropriate strconv functions
pkg/my-sql/adapter_test.go	Updated error message expectation for Int64 parsing
pkg/ldap/datasource.go	Changed from ParseFloat to ParseInt for Int64 attributes
pkg/bamboohr/datasource.go	Separated Int64 and Double parsing logic
pkg/bamboohr/datasource_test.go	Updated test expectations from float64 to int64 values
pkg/aws-s3/csv.go	Added separate Int64 parsing case with appropriate error messaging
pkg/aws-s3/* test files	Updated test data and expectations for int64 handling
go.mod	Updated adapter-framework dependency version

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Saurabhdarekar]

@claude review the PR

[claude]

Claude finished @Saurabhdarekar's task —— View job

---

PR Review: Refactored code to remove explicit conversion

Review Progress:

• Analyze go.mod dependency update
• Review core parsing logic changes in datasource files
• Check test updates for consistency
• Verify error handling improvements
• Provide overall assessment and recommendations

Summary

This PR successfully removes explicit int64 to float64 conversions by updating to adapter-framework v0.18.1-0.20250904145911-30a11c0a6f11. The changes are clean and consistent across all adapters.

Key Changes Reviewed

1. Dependency Update (go.mod:21)

• ✅ Updated to adapter-framework version that supports native int64 processing
• ✅ Change is minimal and focused

2. Core Parsing Logic Improvements

• MySQL (pkg/my-sql/datasource.go:235-236): ✅ Clean separation of Int64 and Double parsing
• BambooHR (pkg/bamboohr/datasource.go:270-273): ✅ Proper separation with correct strconv functions
• LDAP (pkg/ldap/datasource.go:810): ✅ Already using ParseInt correctly
• AWS S3 CSV (pkg/aws-s3/csv.go:266-275): ✅ Well-structured switch cases

3. Test Updates

• ✅ BambooHR tests properly updated from float64 expectations to int64 values
• ✅ AWS S3 tests maintained proper type expectations
• ✅ MySQL error message correctly updated to reference "int64" parsing

Technical Assessment

Strengths:

1. Type Safety: Eliminates ambiguous float64 conversions for integer data
2. Consistency: All adapters follow the same pattern with separate cases for Int64/Double
3. Error Handling: Proper error messages mentioning correct types
4. Test Coverage: Tests updated to match new behavior expectations

Code Quality:

• Clean, readable switch statement structure
• Proper use of strconv.ParseInt(str, 10, 64) vs strconv.ParseFloat(str, 64)
• Error messages are informative and type-specific
• No leftover conversion artifacts

Recommendations

This PR is ready for merge. The implementation is:

• ✅ Functionally correct
• ✅ Well-tested with updated expectations
• ✅ Consistent across all adapters
• ✅ Follows Go best practices
• ✅ Removes code ambiguity as intended

The refactoring successfully addresses the original requirement to remove explicit int64→float64 conversions while maintaining type safety and proper error handling.

---

[nholbrook]

Core logic LGTM. A few nits, but these can be addressed as a followup.

One important point I want to make sure we've discussed before releasing this to PROD: have we done investigation / sufficient testing to ensure this won't cause any regressions?

With this PR, across multiple SoRs, we'll be changing the default behavior for new AND existing SoRs. This means that any existing nodes affected by this change will have the datatype of some of their fields changed (and potentially the value). If we have any customers relying on this field being a float (or if we have any fields that are specified as Ints but actually being provided as Floats, we could see a regression).

I'll raise this as a discussion topic in standup so we determine if we're fine merging this as-is or if we want to change how we roll out this change

[Saurabhdarekar]

There is an investigation needed as Nick mentioned before merging this PR