Teradata initial commit #2
Conversation
…an/dagster into teradata-initial-commit
Teradata initial commit
|
[like] Chinthanippu, Satish reacted to your message:
…________________________________
From: Mohan Talla ***@***.***>
Sent: Tuesday, December 3, 2024 5:28:17 AM
To: Teradata/dagster ***@***.***>
Cc: Chinthanippu, Satish ***@***.***>; Review requested ***@***.***>
Subject: [EXTERNAL] [Teradata/dagster] Teradata initial commit (PR #2)
[CAUTION: External Email]
Added TeradataResource and TeradataComputeClusterManager
________________________________
You can view, comment on, or merge this pull request online at:
#2
Commit Summary
* cab64eb<cab64eb> initial_commit
* 7235731<7235731> Added test case
* d2de755<d2de755> Create integration.yaml
* a7587a2<a7587a2> Merge branch 'dagster-io:master' into teradata-initial-commit
* 74e78c0<74e78c0> next commit
* 9c8cef1<9c8cef1> Merge branch 'teradata-initial-commit' of https://github.com/tallamohan/dagster into teradata-initial-commit
* 05bcb63<05bcb63> Added S3 and Azure
* b1645ad<b1645ad> added mandatory function get_object_to_set_on_execution_context
* 4c525f8<4c525f8> compute cluster create and drop
* 64bd8c9<64bd8c9> Removed sensitive data from config
* a5dfde4<a5dfde4> Added create and drop compute cluster
* 03cf182<03cf182> Removed hard coded values
* fc68012<fc68012> Merge pull request #1 from tallamohan/teradata-initial-commit
* fd1c118<fd1c118> refactored code
* 8ef30e3<8ef30e3> Added handle_cc_status and constants
* 2e32773<2e32773> added TeradataComputeClusterManager
File Changes
(23 files<https://github.com/Teradata/dagster/pull/2/files>)
* A python_modules/libraries/dagster-teradata/.coveragerc<https://github.com/Teradata/dagster/pull/2/files#diff-c6a41ecef58047409fa3c2f342ebd9bc2c8e7a2c86e06c84e7e42a64d2cd7c80> (2)
* A python_modules/libraries/dagster-teradata/LICENSE<https://github.com/Teradata/dagster/pull/2/files#diff-0bcc14a7233eb9368ef6c30494f59bc26b11e6c8c5d80c3b7e7594bed1566355> (201)
* A python_modules/libraries/dagster-teradata/MANIFEST.in<https://github.com/Teradata/dagster/pull/2/files#diff-e9fd57d80fa6b72cb33e625bf1e5deb88cb4651172d8b14112c4731d045dbd4e> (2)
* A python_modules/libraries/dagster-teradata/README.md<https://github.com/Teradata/dagster/pull/2/files#diff-e008d27b6a821f28ca5a8e1d309dc5f37c170cd487a44e1d7cd373ed843c40fb> (4)
* A python_modules/libraries/dagster-teradata/dagster_teradata/__init__.py<https://github.com/Teradata/dagster/pull/2/files#diff-ec346929f4f4ef3bb7d0ff25196a77b0e1187d9aecccd3cf21d8018b4e9ea953> (11)
* A python_modules/libraries/dagster-teradata/dagster_teradata/constants.py<https://github.com/Teradata/dagster/pull/2/files#diff-564c9fd641347e9aacfe3b6ed6f4d28cfbe454d164bdfb4a5692e0a78991061e> (29)
* A python_modules/libraries/dagster-teradata/dagster_teradata/py.typed<https://github.com/Teradata/dagster/pull/2/files#diff-7a207b871d1a560326cc8ae731128f76f640e5fd16b27d5889c4c45e5f3ca9f6> (1)
* A python_modules/libraries/dagster-teradata/dagster_teradata/resources.py<https://github.com/Teradata/dagster/pull/2/files#diff-896081ec11d086441abc62ba4fdbdf9926368fd009bce3c7bf353256f7c957d9> (464)
* A python_modules/libraries/dagster-teradata/dagster_teradata/teradata_compute_cluster_manager.py<https://github.com/Teradata/dagster/pull/2/files#diff-9b9cd6deb9aa1fd8211f1779445cbbec52f4a7005892302471aedcbccc5910a2> (350)
* A python_modules/libraries/dagster-teradata/dagster_teradata/teradata_io_manager.py<https://github.com/Teradata/dagster/pull/2/files#diff-2d790ebb47e47fc2edd40384db801e5550695be9c180c561163c19061e1d3a27> (135)
* A python_modules/libraries/dagster-teradata/dagster_teradata/version.py<https://github.com/Teradata/dagster/pull/2/files#diff-8e3a013d2ac742bf5db80a63e01acbf082c97fd7015d153a393a9e4e3166243b> (1)
* A python_modules/libraries/dagster-teradata/dagster_teradata_tests/__init__.py<https://github.com/Teradata/dagster/pull/2/files#diff-9393153b8a1811627a81f1d179a950b29e5ebfe2d1d14cc7231f30b48705c103> (0)
* A python_modules/libraries/dagster-teradata/dagster_teradata_tests/test_create_teradata_compute_cluster.py<https://github.com/Teradata/dagster/pull/2/files#diff-42315e0c5a6b0c03a78e455c5a2efa359ab4ff68eac9bc6c10db675d724ec37e> (50)
* A python_modules/libraries/dagster-teradata/dagster_teradata_tests/test_drop_teradata_compute_cluster.py<https://github.com/Teradata/dagster/pull/2/files#diff-271d3b12441621b6409e4ca012ec7f08032e684edeba71c6cee705646777ecda> (41)
* A python_modules/libraries/dagster-teradata/dagster_teradata_tests/test_io_manager.py<https://github.com/Teradata/dagster/pull/2/files#diff-13f649231492ccb015298495c26ac3d2a570956f5d5c33fd8e236fa392251736> (167)
* A python_modules/libraries/dagster-teradata/dagster_teradata_tests/test_resources.py<https://github.com/Teradata/dagster/pull/2/files#diff-10f91b265ff5be78c7613646ff8d25d4fea3b453f626427ee24c64f2657258bc> (107)
* A python_modules/libraries/dagster-teradata/dagster_teradata_tests/test_s3_to_teradata_S3Resource.py<https://github.com/Teradata/dagster/pull/2/files#diff-57b6d1c82362140c76dc4f59ac1ef2679bf0bb727c4d2bebe4274e54fbb34876> (34)
* A python_modules/libraries/dagster-teradata/dagster_teradata_tests/test_s3_to_teradata_session.py<https://github.com/Teradata/dagster/pull/2/files#diff-d6ad0bdc049d98ca3c39b0f434082abd2366401a328bd1f95d2746e6aaba70ea> (35)
* A python_modules/libraries/dagster-teradata/integration.yaml<https://github.com/Teradata/dagster/pull/2/files#diff-c1e7319300736ec3cf5ab9c4124adfa38d930c8c94787b41ef27746865ae63e1> (5)
* A python_modules/libraries/dagster-teradata/setup.cfg<https://github.com/Teradata/dagster/pull/2/files#diff-875bc565097dc6deb1f4c6494a99896879392d218e0a32ff1798b29abadaa55a> (7)
* A python_modules/libraries/dagster-teradata/setup.py<https://github.com/Teradata/dagster/pull/2/files#diff-3310f7efa62ba362c4007b8b9b6880f89b36e81bb766cd47ac6133b8a109582b> (44)
* A python_modules/libraries/dagster-teradata/tox.ini<https://github.com/Teradata/dagster/pull/2/files#diff-e242a8eb8b3e351694d186ce0636a1ad318a8d310c4ab6c3556e77489d6b10c7> (22)
* M scripts/install_dev_python_modules.py<https://github.com/Teradata/dagster/pull/2/files#diff-85c2bb6d4721ed0a146ea51e2bd615dbdf2249ad3714adc1cacc1877707cadee> (1)
Patch Links:
* https://github.com/Teradata/dagster/pull/2.patch
* https://github.com/Teradata/dagster/pull/2.diff
—
Reply to this email directly, view it on GitHub<#2>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AFPW3AUV57XG7GSR5S722TT2DU6PDAVCNFSM6AAAAABS46U7L6VHI2DSMVQWIX3LMV43ASLTON2WKOZSG4YTIMBTGU4DQNQ>.
You are receiving this because your review was requested.Message ID: ***@***.***>
|
| @@ -0,0 +1,2 @@ | |||
| [run] | |||
| branch = True | |||
There was a problem hiding this comment.
What is this file used and what's important of this field.
There was a problem hiding this comment.
The .coveragerc file serves as the configuration file for coverage.py. The "branch" option determines whether to measure branch coverage alongside statement coverage. This configuration file is included in all adapters. To maintain consistency with Dagster and other integrations, I have added this file.
| @@ -0,0 +1,2 @@ | |||
| include LICENSE | |||
| include dagster_teradata/py.typed | |||
There was a problem hiding this comment.
py.typed contains only partial. What's the role of this file and why do we require it in MANIFEST file?
There was a problem hiding this comment.
The py.typed file is used in Python projects to indicate that the package supports type annotations and is compliant with PEP 561, which outlines how type information is distributed. This file enables type checkers like mypy to recognize and use the type hints in your package.
By default, non-code files like py.typed are not automatically included in the package distribution. To ensure py.typed is included in the distribution (and therefore available to type checkers when the package is installed), it needs to be explicitly added to the MANIFEST.in file
If the py.typed file contains only the word partial, it indicates that the type annotations provided in the package are incomplete and may not cover all aspects of the API. This serves as a disclaimer for users relying on type checking, making it clear that additional caution or manual checks might be necessary.
This is also assed to maintain consistency with Dagster and other integrations.
| # dagster-teradata | ||
|
|
||
| The docs for `dagster-teradata` can be found | ||
| [here](https://docs.dagster.io/_apidocs/libraries/dagster-teradata). |
There was a problem hiding this comment.
Hope this file will be available after release of teradata?
There was a problem hiding this comment.
It should be ready by the time we release, and the work is progressing in parallel.
| from dagster_teradata.resources import ( | ||
| TeradataConnection as TeradataConnection, | ||
| TeradataResource as TeradataResource, | ||
| fetch_last_updated_timestamps as fetch_last_updated_timestamps, |
There was a problem hiding this comment.
Do we require these imports in this file?
| @@ -0,0 +1 @@ | |||
| __version__ = "1!0+dev" | |||
There was a problem hiding this comment.
Do we need to update this file while release or Dagster team will take care of version change?
| """Loads CSV, JSON, and Parquet format data from Azure Blob Storage to Teradata. | ||
|
|
||
| Args: | ||
| :param blob_source_key: The URI format specifying the location of the Azure blob object store. |
There was a problem hiding this comment.
documentation missed for azure_client_id and azure_client_secret. Rearrange documentation of fields as per method parameters defined order
There was a problem hiding this comment.
I will add that
| from dagster_teradata.teradata_compute_cluster_manager import TeradataComputeClusterManager | ||
|
|
||
|
|
||
| class TeradataResource(ConfigurableResource, IAttachDifferentObjectToOpContext): |
There was a problem hiding this comment.
I feel, this resource can be divided into three parts. One for Handling normal query execution, second one for DataTransfer and other for ComputeCluster. Please think on this
There was a problem hiding this comment.
Sure, will check the possibility
| ) | ||
|
|
||
|
|
||
| def fetch_last_updated_timestamps( |
There was a problem hiding this comment.
What is role of this method?
| config_schema=TeradataResource.to_config_schema(), | ||
| description="This resource is for connecting to the Teradata Vantage", | ||
| ) | ||
| def teradata_resource(context) -> TeradataConnection: |
There was a problem hiding this comment.
Does this method used internal by dagster core framework?
There was a problem hiding this comment.
Yes, this is mandatory
| @@ -0,0 +1,135 @@ | |||
| from abc import abstractmethod | |||
There was a problem hiding this comment.
What is role of this class
There was a problem hiding this comment.
We don't need "teradata_io_manager.py". I will remove that file completely
Added TeradataResource and TeradataComputeClusterManager