-
Notifications
You must be signed in to change notification settings - Fork 41
Env config #509
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
Merged
Merged
Env config #509
Changes from 4 commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
51d492c
Env config
THardy98 5665732
remove merge conflicts
THardy98 b7536f2
feat: Update bridge integration to use new synchronous C API
THardy98 e22dde7
Convert ClientConfig classes to records and improve immutability
THardy98 baf24c0
Remove CLAUDE files
THardy98 535b8a0
Remove useless disableFile parameter when loading ClientConfig. Parse…
THardy98 8c80846
Tests formatting
THardy98 3284607
Address most PR feedback
THardy98 58f06b7
Use scope.ByteArray in favor of sbyte*
THardy98 049b3d0
Deserialize JSON from core using JSON DTO objects
THardy98 8ba428c
Clean up - renaming and using shorthand returns
THardy98 33aadd9
Use renamed core methods with Env prefix before Config. Use tri-state…
THardy98 05d7cf6
Rename ConfigProfile -> Profile, with suppression of build warning
THardy98 28eb602
TlsRecord & ProfileRecord removed, in favor of to/fromDictionary meth…
THardy98 9afb69b
Make to/from dictionary conversion non-null. Minor style improvements
THardy98 0cb850b
Remove runtime usage
THardy98 99ca3e8
Update submodule with merged commit
THardy98 4a621e1
Merge branch 'main' into env_config
THardy98 dd51f40
Formatting
THardy98 a8b4305
Fix interop conflicts
THardy98 f563daa
update test file paths for windows
THardy98 28a93d8
Merge branch 'main' into env_config
THardy98 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
{ | ||
"permissions": { | ||
"allow": [ | ||
"Bash(export PATH=\"$HOME/.dotnet:$PATH\")", | ||
"Bash(dotnet test:*)", | ||
"Bash(dotnet build:*)", | ||
"Read(//Users/thardy/code/sdk-python/tests/**)", | ||
"Read(//Users/thardy/code/sdk-typescript/packages/test/src/**)", | ||
"Read(//Users/thardy/code/sdk-python/temporalio/**)", | ||
"Read(//Users/thardy/code/sdk-typescript/**)", | ||
"Read(//Users/thardy/code/sdk-typescript/**)", | ||
"Read(//Users/thardy/code/**)", | ||
"Bash(git checkout:*)", | ||
"Bash(grep:*)" | ||
], | ||
"deny": [], | ||
"ask": [] | ||
} | ||
} |
THardy98 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
# Environment Configuration Implementation Guidelines for .NET SDK | ||
|
||
## Golden Rule | ||
When unsure about implementation details, ALWAYS ask the developer. | ||
|
||
## Implementation Status | ||
**IMPORTANT**: The .NET SDK environment configuration feature is **NEARLY COMPLETE** and functional. Focus on bug fixes and test improvements rather than reimplementation. | ||
|
||
## Required Implementation Plans | ||
All work on environment configuration MUST follow these plans: | ||
- **Primary**: `@general-implementation-plan.md` - Overall architecture and cross-SDK consistency | ||
- **Secondary**: `@dotnet-implementation-plan.md` - .NET-specific implementation details | ||
|
||
## Current Implementation Analysis | ||
|
||
### ✅ COMPLETED Components | ||
1. **C Bridge Layer** (`src/Temporalio/Bridge/sdk-core/core-c-bridge/src/envconfig.rs`) | ||
- `temporal_core_client_config_load` - Loads all profiles | ||
- `temporal_core_client_config_profile_load` - Loads single profile | ||
- Proper memory management and error handling | ||
- JSON serialization between Rust and C | ||
|
||
2. **P/Invoke Bridge** (`src/Temporalio/Bridge/EnvConfig.cs`) | ||
- Complete async/await implementation | ||
- Proper callback handling and memory cleanup | ||
- JSON deserialization to .NET objects | ||
|
||
3. **API Surface** (`src/Temporalio/Client/Configuration/`) | ||
- `ClientConfig` - Main configuration container | ||
- `ClientConfigProfile` - Profile-specific settings | ||
- `ClientConfigTls` - TLS configuration | ||
- `DataSource` - Flexible data source abstraction | ||
- Integration with `TemporalClientConnectOptions` | ||
|
||
## Implementation Plans | ||
All work on environment configuration MUST follow these detailed plans: | ||
- **Primary**: `@general-implementation-plan.md` - Overall architecture and cross-SDK consistency | ||
- **Secondary**: `@dotnet-implementation-plan.md` - .NET-specific implementation details and current status | ||
- **Assessment**: `@envconfig-assessment.md` - Comprehensive cross-SDK analysis and comparison | ||
|
||
## Current Status Summary | ||
The .NET SDK implementation is **NEARLY COMPLETE** and functional, requiring only: | ||
1. **Critical bug fix** for TLS field name mismatch (see dotnet-implementation-plan.md) | ||
2. **Comprehensive test coverage** following Python/TypeScript patterns | ||
3. **Validation** of bridge layer functionality | ||
|
||
See `@dotnet-implementation-plan.md` for detailed status, issues, and completion steps. |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.