Refactor: moving code specific to miner from redshift to application-sdk #742
+31
−4
Conversation
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
- preflight_check - get_workflow_args
abhishekagrawal-atlan
requested review from
firecast and
inishchith
as code owners
📜 Docstring Coverage ReportRESULT: PASSED (minimum: 30.0%, actual: 75.6%) Detailed Coverage Report |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors code by moving miner-specific functions from redshift-app to application-sdk, making them accessible for broader use across different applications.
- Adds preflight_check and get_workflow_args methods to the SQL query extraction activity class
- Updates read_marker method to accept an optional output_path parameter
- Imports additional utility for credential parsing
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
📦 Trivy Vulnerability Scan Results
Report Summary
Scan Result Details✅ No vulnerabilities found during the scan for |
📦 Trivy Secret Scan Results
Report Summary
Scan Result Details✅ No secrets found during the scan for |
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
|
🛠 Full Test Coverage Report: https://k.atlan.dev/coverage/application-sdk/pr/742 |
|
🛠 Docs available at: https://k.atlan.dev/application-sdk/miner-refactor |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
inishchith
reviewed
Sep 30, 2025
|
|
||
| @activity.defn(name="miner_get_workflow_args") | ||
| @auto_heartbeater | ||
| async def get_workflow_args(self, workflow_config: Dict[str, Any]): |
There was a problem hiding this comment.
this should not have any code other than the super()...
| resolved_output_path: str = ( | ||
| output_path | ||
| if isinstance(output_path, str) and output_path | ||
| else base_output_path.rsplit("/", 1)[0] |
There was a problem hiding this comment.
this line can be made more readable. Either via using constants that tell what is at 1 and 0 index or via a comment stating 'why' this is being done - expected output.
| @auto_heartbeater | ||
| async def get_workflow_args(self, workflow_config: Dict[str, Any]): | ||
| workflow_args = await super().get_workflow_args(workflow_config) | ||
| if "credential_guid" in workflow_args: |
There was a problem hiding this comment.
nitpick - prefer using constants/variables when there is repeat use of key strings in a code snippet.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Changelog
Additional context (e.g. screenshots, logs, links)
Checklist
Copyleft License Compliance
Note
Adds miner preflight and workflow-args activities, enables marker reading with optional output path, and sets deployment_type from credential extras.
miner_preflight_checkandminer_get_workflow_argsactivities delegating to base implementations.miner_get_workflow_args: fetches credentials and setsworkflow_args["deployment_type"]from parsed credentialextra(defaultprovisioned).read_markernow accepts optionaloutput_path, resolves path robustly, and uses stricter typing.castandparse_credentials_extrafor the above features.Written by Cursor Bugbot for commit 25bcb50. This will update automatically on new commits. Configure here.