Feat: Make CDK runnable in WASM #672
Conversation
…l for WASM compatibility - Remove psutil and rapidfuzz from main dependencies (marked as unused) - Move google-cloud-secret-manager to dev extras group - Add new 'system' extras group for psutil and rapidfuzz - Update deptry configuration comments to reflect optional status This enables airbyte-cdk to be installed in WASM environments like Pyodide by making problematic binary dependencies optional. Co-Authored-By: AJ Steers <[email protected]>
Automatic code formatting applied during dependency update process. Includes line wrapping and copyright header removal. Co-Authored-By: AJ Steers <[email protected]>
Address PR feedback from @aaronsteers to fully remove psutil and rapidfuzz rather than making them optional dependencies. These packages are unused and were marked as TODO for removal. Changes: - Remove psutil and rapidfuzz from optional dependencies - Remove 'system' extras group (no longer needed) - Update deptry comments to reflect complete removal - Keep google-cloud-secret-manager in dev extras as requested Co-Authored-By: AJ Steers <[email protected]>
- Add conditional dependencies in pyproject.toml using environment markers - Use sys_platform != 'emscripten' for serpyco-rs, sys_platform == 'emscripten' for serpyco - Update imports in airbyte_protocol_serializers.py, airbyte_protocol.py, entrypoint_wrapper.py, and test_source.py - Add conditional import logic to detect WASM/Pyodide environments at runtime - Maintain API compatibility between serpyco-rs and serpyco libraries - All tests pass with conditional imports Co-Authored-By: AJ Steers <[email protected]>
…ity-deps' into aj/feat/run-cdk-in-wasm
aaronsteers
changed the base branch from
devin/1753304881-serpyco-wasm-fallback
to
main
There was a problem hiding this comment.
Pull Request Overview
This PR makes the CDK (Connector Development Kit) runnable in WebAssembly (WASM) by removing dependencies that are incompatible with WASM environments and making others optional.
- Removes WASM-incompatible dependencies:
psutilandrapidfuzz - Makes
google-cloud-secret-manageran optional dependency in the 'dev' extra - Updates dependency configuration and comments to reflect WASM compatibility changes
| @@ -39,20 +39,18 @@ click = "^8.1.8" | |||
| dpath = "^2.1.6" | |||
| dunamai = "^1.22.0" | |||
| genson = "1.3.0" | |||
| google-cloud-secret-manager = "^2.17.0" | |||
| google-cloud-secret-manager = { version = "^2.17.0", optional = true } # 'dev' extra | |||
There was a problem hiding this comment.
[nitpick] The comment "'dev' extra" could be clearer. Consider using "Optional dependency included in 'dev' extra" to better explain its purpose and usage.
| google-cloud-secret-manager = { version = "^2.17.0", optional = true } # 'dev' extra | |
| google-cloud-secret-manager = { version = "^2.17.0", optional = true } # Optional dependency included in 'dev' extra |
Copilot uses AI. Check for mistakes.
| @@ -230,14 +228,12 @@ DEP001 = [ | |||
| DEP002 = [ | |||
| "setuptools", # Incorrectly used without declaring in some connectors (e.g. source-shopify) | |||
| "cryptography", # Constrained as transitive dependency due to a bug in newer versions | |||
| "google-cloud-secret-manager", # Deptry can't detect that `google.cloud.secretmanager_v1` uses this package | |||
| "google-cloud-secret-manager", # Deptry can't detect that `google.cloud.secretmanager_v1` uses this package - optional 'dev' dependency | |||
There was a problem hiding this comment.
[nitpick] The comment combines two different pieces of information. Consider splitting into two comments or restructuring: "Deptry can't detect that google.cloud.secretmanager_v1 uses this package. Now optional 'dev' dependency for WASM compatibility."
| "google-cloud-secret-manager", # Deptry can't detect that `google.cloud.secretmanager_v1` uses this package - optional 'dev' dependency | |
| "google-cloud-secret-manager", # Deptry can't detect that `google.cloud.secretmanager_v1` uses this package. | |
| # This is an optional 'dev' dependency for WASM compatibility. |
Copilot uses AI. Check for mistakes.
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. Testing This CDK VersionYou can test this version of the CDK using the following: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@aj/feat/run-cdk-in-wasm#egg=airbyte-python-cdk[dev]' --help
# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch aj/feat/run-cdk-in-wasmHelpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
PyTest Results (Fast)1 tests - 3 699 0 ✅ - 3 689 2s ⏱️ - 6m 42s For more details on these errors, see this check. Results for commit b90f3f5. ± Comparison against base commit 51cfea5. This pull request removes 3700 and adds 1 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
📝 WalkthroughWalkthroughThis update introduces conditional imports throughout the codebase to select between the Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant sys
participant serpyco_rs
participant serpyco
App->>sys: Check platform (sys.platform)
alt platform != "emscripten"
App->>serpyco_rs: Import modules/classes
else platform == "emscripten"
App->>serpyco: Import modules/classes
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Would you like to consider adding a brief comment near each conditional import to clarify the rationale for future maintainers, or do you prefer to keep the code concise as is? Wdyt? Suggested reviewers
Note ⚡️ Unit Test Generation - BetaCodeRabbit's unit test generation is now available in Beta! Automatically generate comprehensive unit tests for your code changes, ensuring better test coverage and catching edge cases you might miss. Our AI analyzes your code structure and creates tests that follow best practices and your project's testing patterns. Learn more here, or just try it under ✨ Finishing Touches. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
pyproject.toml (1)
231-231: Consider improving comment clarity as suggested previously.The past review comment about splitting this into two separate comments still applies. What do you think about making it clearer by separating the deptry detection issue from the WASM compatibility note, wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
airbyte_cdk/models/airbyte_protocol.py(1 hunks)airbyte_cdk/models/airbyte_protocol_serializers.py(2 hunks)airbyte_cdk/test/entrypoint_wrapper.py(2 hunks)pyproject.toml(4 hunks)unit_tests/sources/test_source.py(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: ChristoGrab
PR: airbytehq/airbyte-python-cdk#58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the `YamlDeclarativeSource` class in `airbyte_cdk/sources/declarative/yaml_declarative_source.py`, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
Learnt from: aaronsteers
PR: airbytehq/airbyte-python-cdk#58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in `airbyte_cdk/cli/source_declarative_manifest/`, including `_run.py`, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.
airbyte_cdk/models/airbyte_protocol.py (4)
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in airbyte_cdk/cli/source_declarative_manifest/, including _run.py, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/spec.json:9-15
Timestamp: 2024-11-15T00:59:08.154Z
Learning: When code in airbyte_cdk/cli/source_declarative_manifest/ is being imported from another repository, avoid suggesting modifications to it during the import process.
Learnt from: ChristoGrab
PR: #58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the YamlDeclarativeSource class in airbyte_cdk/sources/declarative/yaml_declarative_source.py, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
Learnt from: aaronsteers
PR: #174
File: airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py:1093-1102
Timestamp: 2025-01-14T00:20:32.310Z
Learning: In the airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py file, the strict module name checks in _get_class_from_fully_qualified_class_name (requiring module_name to be "components" and module_name_full to be "source_declarative_manifest.components") are intentionally designed to provide early, clear feedback when class declarations won't be found later in execution. These restrictions may be loosened in the future if the requirements for class definition locations change.
airbyte_cdk/test/entrypoint_wrapper.py (4)
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in airbyte_cdk/cli/source_declarative_manifest/, including _run.py, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/spec.json:9-15
Timestamp: 2024-11-15T00:59:08.154Z
Learning: When code in airbyte_cdk/cli/source_declarative_manifest/ is being imported from another repository, avoid suggesting modifications to it during the import process.
Learnt from: ChristoGrab
PR: #58
File: airbyte_cdk/sources/declarative/yaml_declarative_source.py:0-0
Timestamp: 2024-11-18T23:40:06.391Z
Learning: When modifying the YamlDeclarativeSource class in airbyte_cdk/sources/declarative/yaml_declarative_source.py, avoid introducing breaking changes like altering method signatures within the scope of unrelated PRs. Such changes should be addressed separately to minimize impact on existing implementations.
Learnt from: pnilan
PR: airbytehq/airbyte-python-cdk#0
File: :0-0
Timestamp: 2024-12-11T16:34:46.319Z
Learning: In the airbytehq/airbyte-python-cdk repository, the declarative_component_schema.py file is auto-generated from declarative_component_schema.yaml and should be ignored in the recommended reviewing order.
unit_tests/sources/test_source.py (2)
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/spec.json:9-15
Timestamp: 2024-11-15T00:59:08.154Z
Learning: When code in airbyte_cdk/cli/source_declarative_manifest/ is being imported from another repository, avoid suggesting modifications to it during the import process.
Learnt from: ChristoGrab
PR: #221
File: airbyte_cdk/sources/utils/transform.py:0-0
Timestamp: 2025-01-16T00:50:39.069Z
Learning: In the TypeTransformer class, the data being transformed comes from API responses or source systems, so only standard JSON-serializable types are expected. The python_to_json mapping covers all expected types, and it's designed to fail fast (KeyError) on unexpected custom types rather than providing fallbacks.
airbyte_cdk/models/airbyte_protocol_serializers.py (3)
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in airbyte_cdk/cli/source_declarative_manifest/, including _run.py, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/spec.json:9-15
Timestamp: 2024-11-15T00:59:08.154Z
Learning: When code in airbyte_cdk/cli/source_declarative_manifest/ is being imported from another repository, avoid suggesting modifications to it during the import process.
Learnt from: ChristoGrab
PR: #221
File: airbyte_cdk/sources/utils/transform.py:0-0
Timestamp: 2025-01-16T00:50:39.069Z
Learning: In the TypeTransformer class, the data being transformed comes from API responses or source systems, so only standard JSON-serializable types are expected. The python_to_json mapping covers all expected types, and it's designed to fail fast (KeyError) on unexpected custom types rather than providing fallbacks.
pyproject.toml (1)
Learnt from: aaronsteers
PR: #58
File: airbyte_cdk/cli/source_declarative_manifest/_run.py:62-65
Timestamp: 2024-11-15T01:04:21.272Z
Learning: The files in airbyte_cdk/cli/source_declarative_manifest/, including _run.py, are imported from another repository, and changes to these files should be minimized or avoided when possible to maintain consistency.
🪛 GitHub Actions: Linters
airbyte_cdk/models/airbyte_protocol.py
[error] 8-8: ruff formatting check failed: inconsistent use of single vs double quotes around 'emscripten'. Run 'ruff format --fix' to fix code style issues.
airbyte_cdk/test/entrypoint_wrapper.py
[error] 30-30: ruff formatting check failed: inconsistent use of single vs double quotes around 'emscripten'. Run 'ruff format --fix' to fix code style issues.
unit_tests/sources/test_source.py
[error] 12-12: ruff formatting check failed: inconsistent use of single vs double quotes around 'emscripten'. Run 'ruff format --fix' to fix code style issues.
airbyte_cdk/models/airbyte_protocol_serializers.py
[error] 23-23: mypy: Name "CustomType" already defined (possibly by an import) [no-redef]
[error] 23-23: mypy: Name "Serializer" already defined (possibly by an import) [no-redef]
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Build Python Package
- GitHub Check: Check: source-intercom
- GitHub Check: Check: destination-motherduck
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: source-shopify
- GitHub Check: Check: source-pokeapi
- GitHub Check: Pytest (Fast)
- GitHub Check: preview_docs
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: SDM Docker Image Build
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Analyze (python)
🔇 Additional comments (5)
pyproject.toml (1)
86-87: Great approach for WASM compatibility!The platform-specific dependency setup using markers is exactly what's needed to support both regular Python environments and WebAssembly. The conditional logic here aligns perfectly with the runtime platform detection in the code files.
airbyte_cdk/test/entrypoint_wrapper.py (1)
20-20: Nice addition for platform detection!The sys import is properly placed and will be used for the conditional import logic below.
airbyte_cdk/models/airbyte_protocol.py (1)
5-5: Good addition for platform detection.The sys import is correctly placed for the conditional logic below.
unit_tests/sources/test_source.py (1)
7-7: Proper sys import for platform detection.Good placement of the sys import for the conditional logic.
airbyte_cdk/models/airbyte_protocol_serializers.py (1)
2-2: Good sys import for platform detection.Properly placed import for the conditional logic.
| USE_RUST_BACKEND = sys.platform != "emscripten" | ||
| """When run in WASM, use the pure Python backend for serpyco.""" | ||
|
|
||
| if USE_RUST_BACKEND: | ||
| from serpyco_rs import CustomType, Serializer | ||
| else: | ||
| from serpyco import CustomType, Serializer | ||
|
|
There was a problem hiding this comment.
Excellent approach with the constant, but fix mypy issue.
I love the USE_RUST_BACKEND constant approach - it makes the code much more readable than inline platform checks! However, mypy is complaining about the name redefinition in the conditional imports.
Could you fix the mypy issue by using type: ignore comments or restructuring the imports, wdyt?
if USE_RUST_BACKEND:
- from serpyco_rs import CustomType, Serializer
+ from serpyco_rs import CustomType, Serializer # type: ignore[misc]
else:
- from serpyco import CustomType, Serializer
+ from serpyco import CustomType, Serializer # type: ignore[misc]Alternatively, you could import with aliases and then assign to common names:
if USE_RUST_BACKEND:
- from serpyco_rs import CustomType, Serializer
+ from serpyco_rs import CustomType as _CustomType, Serializer as _Serializer
else:
- from serpyco import CustomType, Serializer
+ from serpyco import CustomType as _CustomType, Serializer as _Serializer
+
+CustomType = _CustomType
+Serializer = _Serializer📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| USE_RUST_BACKEND = sys.platform != "emscripten" | |
| """When run in WASM, use the pure Python backend for serpyco.""" | |
| if USE_RUST_BACKEND: | |
| from serpyco_rs import CustomType, Serializer | |
| else: | |
| from serpyco import CustomType, Serializer | |
| USE_RUST_BACKEND = sys.platform != "emscripten" | |
| """When run in WASM, use the pure Python backend for serpyco.""" | |
| if USE_RUST_BACKEND: | |
| from serpyco_rs import CustomType, Serializer # type: ignore[misc] | |
| else: | |
| from serpyco import CustomType, Serializer # type: ignore[misc] |
| USE_RUST_BACKEND = sys.platform != "emscripten" | |
| """When run in WASM, use the pure Python backend for serpyco.""" | |
| if USE_RUST_BACKEND: | |
| from serpyco_rs import CustomType, Serializer | |
| else: | |
| from serpyco import CustomType, Serializer | |
| USE_RUST_BACKEND = sys.platform != "emscripten" | |
| """When run in WASM, use the pure Python backend for serpyco.""" | |
| if USE_RUST_BACKEND: | |
| from serpyco_rs import CustomType as _CustomType, Serializer as _Serializer | |
| else: | |
| from serpyco import CustomType as _CustomType, Serializer as _Serializer | |
| CustomType = _CustomType | |
| Serializer = _Serializer |
🧰 Tools
🪛 GitHub Actions: Linters
[error] 23-23: mypy: Name "CustomType" already defined (possibly by an import) [no-redef]
[error] 23-23: mypy: Name "Serializer" already defined (possibly by an import) [no-redef]
🤖 Prompt for AI Agents
In airbyte_cdk/models/airbyte_protocol_serializers.py around lines 17 to 24,
mypy raises an error due to the redefinition of CustomType and Serializer in the
conditional imports. To fix this, import the modules with distinct aliases based
on the condition and then assign the common names CustomType and Serializer to
the appropriate aliases. This avoids name redefinition and satisfies mypy
without losing readability.
| if sys.platform == 'emscripten': | ||
| from serpyco.metadata import Alias | ||
| else: | ||
| from serpyco_rs.metadata import Alias |
There was a problem hiding this comment.
Fix quote consistency for pipeline compliance.
The conditional import approach for the Alias class is perfect for WASM support! Just need to fix the quote style to match the pipeline requirements.
Could you update the quote style to be consistent, wdyt?
-if sys.platform == 'emscripten':
+if sys.platform == "emscripten":📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if sys.platform == 'emscripten': | |
| from serpyco.metadata import Alias | |
| else: | |
| from serpyco_rs.metadata import Alias | |
| if sys.platform == "emscripten": | |
| from serpyco.metadata import Alias | |
| else: | |
| from serpyco_rs.metadata import Alias |
🤖 Prompt for AI Agents
In airbyte_cdk/models/airbyte_protocol.py around lines 11 to 14, the quote style
used in the conditional import statement is inconsistent. Update all string
literals in this block to use the same type of quotes (either single or double)
consistently to comply with the pipeline's style requirements.
| if sys.platform == 'emscripten': | ||
| from serpyco import SchemaValidationError | ||
| else: | ||
| from serpyco_rs import SchemaValidationError |
There was a problem hiding this comment.
Fix quote consistency for pipeline compliance.
The conditional import logic is solid for WASM compatibility! However, there's a formatting issue with quote consistency that's causing the pipeline to fail.
Could you apply this fix to match the codebase's quote style, wdyt?
-if sys.platform == 'emscripten':
+if sys.platform == "emscripten":📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if sys.platform == 'emscripten': | |
| from serpyco import SchemaValidationError | |
| else: | |
| from serpyco_rs import SchemaValidationError | |
| if sys.platform == "emscripten": | |
| from serpyco import SchemaValidationError | |
| else: | |
| from serpyco_rs import SchemaValidationError |
🤖 Prompt for AI Agents
In airbyte_cdk/test/entrypoint_wrapper.py around lines 33 to 36, the import
statements use inconsistent quote styles. To fix the pipeline failure caused by
this, update the quotes in both import lines to match the project's preferred
quote style (either single or double quotes consistently). Ensure both 'serpyco'
and 'serpyco_rs' imports use the same type of quotes.
unit_tests/sources/test_source.py
Outdated
| if sys.platform == 'emscripten': | ||
| from serpyco import SchemaValidationError | ||
| else: | ||
| from serpyco_rs import SchemaValidationError |
There was a problem hiding this comment.
Great test compatibility approach, but fix quotes.
The conditional import ensures tests work correctly with both serialization backends depending on the platform. Just need to fix the quote consistency for pipeline compliance.
Could you update to use double quotes, wdyt?
-if sys.platform == 'emscripten':
+if sys.platform == "emscripten":📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if sys.platform == 'emscripten': | |
| from serpyco import SchemaValidationError | |
| else: | |
| from serpyco_rs import SchemaValidationError | |
| if sys.platform == "emscripten": | |
| from serpyco import SchemaValidationError | |
| else: | |
| from serpyco_rs import SchemaValidationError |
🤖 Prompt for AI Agents
In unit_tests/sources/test_source.py around lines 15 to 18, the import
statements use single quotes for the platform string and module names. Update
all single quotes to double quotes for consistency and pipeline compliance,
changing 'emscripten' to "emscripten" and similarly for the module names in the
import statements.
PyTest Results (Full)15 tests - 3 688 0 ✅ - 3 692 14s ⏱️ - 17m 57s For more details on these errors, see this check. Results for commit b90f3f5. ± Comparison against base commit 51cfea5. This pull request removes 3703 and adds 15 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Combines:
Summary by CodeRabbit
Bug Fixes
Chores