Skip to content

Oracle Database support #1259

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 2 commits into from
Jun 22, 2022
Merged

Oracle Database support #1259

merged 2 commits into from
Jun 22, 2022

Conversation

cnfait
Copy link
Contributor

@cnfait cnfait commented Apr 6, 2022

Feature or Bugfix

  • Feature

Detail

  • Oracle Database support

Relates

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@cnfait cnfait added the enhancement New feature or request label Apr 6, 2022
@cnfait cnfait self-assigned this Apr 6, 2022
@jaidisido
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
  • Commit ID: 46c2db9
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@jaidisido
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
  • Commit ID: c389d13
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@jaidisido
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
  • Commit ID: dc77396
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@jaidisido
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
  • Commit ID: f7aafe1
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@cnfait cnfait force-pushed the oracle-support branch 3 times, most recently from 142179f to bdfc717 Compare April 12, 2022 13:19
@cnfait cnfait force-pushed the oracle-support branch 3 times, most recently from 7772e74 to ea0668f Compare April 14, 2022 10:13
@jaidisido
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
  • Commit ID: d4e39ee
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@jaidisido
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
  • Commit ID: af0519b
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@jaidisido
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
  • Commit ID: ed33f9e
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@cnfait cnfait marked this pull request as ready for review April 28, 2022 09:52
Copy link
Contributor

@jaidisido jaidisido left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall, left a few comments

@@ -0,0 +1,214 @@
import logging
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, can we quickly check the coverage score for these tests?

If you run a similar command: https://github.com/awslabs/aws-data-wrangler/blob/main/tox.ini#L26 via tox, you should obtain a report. Mostly interested in the Oracle section

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is what I get for the coverage:

Name                                    Stmts   Miss  Cover
-----------------------------------------------------------
awswrangler/__init__.py                     6      0   100%
awswrangler/__metadata__.py                 4      0   100%
awswrangler/_config.py                    269     37    86%
awswrangler/_data_types.py                570    460    19%
awswrangler/_databases.py                 141     95    33%
awswrangler/_utils.py                     196     71    64%
awswrangler/athena/__init__.py              3      0   100%
awswrangler/athena/_cache.py              114     80    30%
awswrangler/athena/_read.py               208    149    28%
awswrangler/athena/_utils.py              292    180    38%
awswrangler/catalog/__init__.py             6      0   100%
awswrangler/catalog/_add.py                48     22    54%
awswrangler/catalog/_create.py            198    123    38%
awswrangler/catalog/_definitions.py        50     32    36%
awswrangler/catalog/_delete.py             48     22    54%
awswrangler/catalog/_get.py               228    145    36%
awswrangler/catalog/_utils.py              83     19    77%
awswrangler/chime.py                       18      8    56%
awswrangler/cloudwatch.py                  62     48    23%
awswrangler/data_api/__init__.py            2      0   100%
awswrangler/data_api/connector.py          29      5    83%
awswrangler/data_api/rds.py                63     11    83%
awswrangler/data_api/redshift.py           75     10    87%
awswrangler/dynamodb/__init__.py            4      0   100%
awswrangler/dynamodb/_delete.py            15      0   100%
awswrangler/dynamodb/_utils.py             13      0   100%
awswrangler/dynamodb/_write.py             32      9    72%
awswrangler/emr.py                        189     49    74%
awswrangler/exceptions.py                  29      0   100%
awswrangler/lakeformation/__init__.py       3      0   100%
awswrangler/lakeformation/_read.py         59     41    31%
awswrangler/lakeformation/_utils.py       125     97    22%
awswrangler/mysql.py                       87     64    26%
awswrangler/neptune/__init__.py             3      3     0%
awswrangler/neptune/_utils.py              58     58     0%
awswrangler/neptune/client.py             134    134     0%
awswrangler/neptune/gremlin_parser.py      34     34     0%
awswrangler/neptune/neptune.py            150    150     0%
awswrangler/opensearch/__init__.py          4      0   100%
awswrangler/opensearch/_read.py            55     44    20%
awswrangler/opensearch/_utils.py           38     25    34%
awswrangler/opensearch/_write.py          171    139    19%
awswrangler/oracle.py                      99     59    40%
awswrangler/postgresql.py                  82     60    27%
awswrangler/quicksight/__init__.py          6      0   100%
awswrangler/quicksight/_cancel.py          16      9    44%
awswrangler/quicksight/_create.py          90     75    17%
awswrangler/quicksight/_delete.py          73     58    21%
awswrangler/quicksight/_describe.py        56     45    20%
awswrangler/quicksight/_get_list.py        93     64    31%
awswrangler/quicksight/_utils.py           20     12    40%
awswrangler/redshift.py                   291    240    18%
awswrangler/s3/__init__.py                 16      0   100%
awswrangler/s3/_copy.py                    71     25    65%
awswrangler/s3/_delete.py                  61      9    85%
awswrangler/s3/_describe.py                48      2    96%
awswrangler/s3/_download.py                16      0   100%
awswrangler/s3/_fs.py                     316    106    66%
awswrangler/s3/_list.py                   117     21    82%
awswrangler/s3/_merge_upsert_table.py      38     10    74%
awswrangler/s3/_read.py                   106     44    58%
awswrangler/s3/_read_excel.py              14      6    57%
awswrangler/s3/_read_parquet.py           280    181    35%
awswrangler/s3/_read_text.py               76     19    75%
awswrangler/s3/_select.py                  58     42    28%
awswrangler/s3/_upload.py                  16      0   100%
awswrangler/s3/_wait.py                    35     23    34%
awswrangler/s3/_write.py                   51     22    57%
awswrangler/s3/_write_concurrent.py        35     13    63%
awswrangler/s3/_write_dataset.py           97     57    41%
awswrangler/s3/_write_excel.py             15      7    53%
awswrangler/s3/_write_parquet.py          151     53    65%
awswrangler/s3/_write_text.py             200    122    39%
awswrangler/secretsmanager.py              17      4    76%
awswrangler/sqlserver.py                   97     61    37%
awswrangler/sts.py                         15      5    67%
awswrangler/timestream.py                 133    105    21%
-----------------------------------------------------------
TOTAL                                    6821   3923    42%```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something is off - the latest coverage report we ran is this one:

Module | statements | missing | excluded | coverage
-- | -- | -- | -- | --
awswrangler/__init__.py | 6 | 0 | 0 | 100%
awswrangler/__metadata__.py | 4 | 0 | 0 | 100%
awswrangler/_config.py | 269 | 12 | 0 | 96%
awswrangler/_data_types.py | 540 | 85 | 0 | 84%
awswrangler/_databases.py | 128 | 11 | 0 | 91%
awswrangler/_utils.py | 196 | 21 | 0 | 89%
awswrangler/athena/__init__.py | 3 | 0 | 0 | 100%
awswrangler/athena/_cache.py | 114 | 4 | 0 | 96%
awswrangler/athena/_read.py | 208 | 28 | 0 | 87%
awswrangler/athena/_utils.py | 292 | 18 | 0 | 94%
awswrangler/catalog/__init__.py | 6 | 0 | 0 | 100%
awswrangler/catalog/_add.py | 48 | 5 | 0 | 90%
awswrangler/catalog/_create.py | 198 | 10 | 0 | 95%
awswrangler/catalog/_definitions.py | 50 | 1 | 0 | 98%
awswrangler/catalog/_delete.py | 48 | 4 | 0 | 92%
awswrangler/catalog/_get.py | 228 | 18 | 0 | 92%
awswrangler/catalog/_utils.py | 83 | 3 | 0 | 96%
awswrangler/chime.py | 18 | 8 | 0 | 56%
awswrangler/cloudwatch.py | 62 | 4 | 0 | 94%
awswrangler/data_api/__init__.py | 2 | 0 | 0 | 100%
awswrangler/data_api/connector.py | 29 | 4 | 0 | 86%
awswrangler/data_api/rds.py | 62 | 12 | 0 | 81%
awswrangler/data_api/redshift.py | 74 | 4 | 0 | 95%
awswrangler/dynamodb/__init__.py | 4 | 0 | 0 | 100%
awswrangler/dynamodb/_delete.py | 15 | 0 | 0 | 100%
awswrangler/dynamodb/_utils.py | 13 | 0 | 0 | 100%
awswrangler/dynamodb/_write.py | 32 | 9 | 0 | 72%
awswrangler/emr.py | 189 | 13 | 0 | 93%
awswrangler/exceptions.py | 29 | 0 | 0 | 100%
awswrangler/lakeformation/__init__.py | 3 | 0 | 0 | 100%
awswrangler/lakeformation/_read.py | 59 | 0 | 0 | 100%
awswrangler/lakeformation/_utils.py | 125 | 10 | 0 | 92%
awswrangler/mysql.py | 87 | 3 | 0 | 97%
awswrangler/neptune/__init__.py | 3 | 0 | 0 | 100%
awswrangler/neptune/_utils.py | 58 | 58 | 0 | 0%
awswrangler/neptune/client.py | 131 | 9 | 0 | 93%
awswrangler/neptune/gremlin_parser.py | 34 | 5 | 0 | 85%
awswrangler/neptune/neptune.py | 135 | 9 | 0 | 93%
awswrangler/opensearch/__init__.py | 4 | 0 | 0 | 100%
awswrangler/opensearch/_read.py | 55 | 4 | 0 | 93%
awswrangler/opensearch/_utils.py | 38 | 6 | 0 | 84%
awswrangler/opensearch/_write.py | 171 | 48 | 0 | 72%
awswrangler/postgresql.py | 79 | 3 | 0 | 96%
awswrangler/quicksight/__init__.py | 6 | 0 | 0 | 100%
awswrangler/quicksight/_cancel.py | 16 | 1 | 0 | 94%
awswrangler/quicksight/_create.py | 90 | 5 | 0 | 94%
awswrangler/quicksight/_delete.py | 73 | 25 | 0 | 66%
awswrangler/quicksight/_describe.py | 56 | 13 | 0 | 77%
awswrangler/quicksight/_get_list.py | 93 | 14 | 0 | 85%
awswrangler/quicksight/_utils.py | 20 | 1 | 0 | 95%
awswrangler/redshift.py | 291 | 16 | 0 | 95%
awswrangler/s3/__init__.py | 16 | 0 | 0 | 100%
awswrangler/s3/_copy.py | 71 | 0 | 0 | 100%
awswrangler/s3/_delete.py | 61 | 8 | 0 | 87%
awswrangler/s3/_describe.py | 48 | 0 | 0 | 100%
awswrangler/s3/_download.py | 16 | 0 | 0 | 100%
awswrangler/s3/_fs.py | 315 | 22 | 0 | 93%
awswrangler/s3/_list.py | 117 | 10 | 0 | 91%
awswrangler/s3/_merge_upsert_table.py | 38 | 0 | 0 | 100%
awswrangler/s3/_read.py | 106 | 5 | 0 | 95%
awswrangler/s3/_read_excel.py | 14 | 1 | 0 | 93%
awswrangler/s3/_read_parquet.py | 280 | 38 | 0 | 86%
awswrangler/s3/_read_text.py | 70 | 3 | 0 | 96%
awswrangler/s3/_select.py | 61 | 6 | 0 | 90%
awswrangler/s3/_upload.py | 16 | 0 | 0 | 100%
awswrangler/s3/_wait.py | 35 | 23 | 0 | 34%
awswrangler/s3/_write.py | 51 | 4 | 0 | 92%
awswrangler/s3/_write_concurrent.py | 35 | 0 | 0 | 100%
awswrangler/s3/_write_dataset.py | 97 | 9 | 0 | 91%
awswrangler/s3/_write_excel.py | 15 | 1 | 0 | 93%
awswrangler/s3/_write_parquet.py | 151 | 2 | 0 | 99%
awswrangler/s3/_write_text.py | 200 | 31 | 0 | 84%
awswrangler/secretsmanager.py | 17 | 1 | 0 | 94%
awswrangler/sqlserver.py | 95 | 4 | 0 | 96%
awswrangler/sts.py | 15 | 0 | 0 | 100%
awswrangler/timestream.py | 133 | 8 | 0 | 94%
Total | 6650 | 680 | 0 | 90%

Overall it's at 90%. Your Oracle tests are similar to SQLServer which has a coverage of 96%. Somehow some of the tests must not have run perhaps?

@cnfait cnfait force-pushed the oracle-support branch from ed33f9e to 464bcce Compare May 5, 2022 09:41
@jaidisido
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubCodeBuild8756EF16-pDO66x4b9gEu
  • Commit ID: 464bcce
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@cnfait cnfait force-pushed the oracle-support branch from 464bcce to 94be9d1 Compare May 9, 2022 14:38
@cnfait cnfait force-pushed the oracle-support branch 3 times, most recently from af25c2b to 87f22b9 Compare June 9, 2022 05:16


def test_to_sql_cast(oracle_table, oracle_con):
table = oracle_table
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor but what is that for?

con = wr.oracle.connect(secret_id="aws-data-wrangler/oracle", dbname=dbname)
df = wr.oracle.read_sql_query("SELECT 1 FROM DUAL", con=con)
assert df.shape == (1, 1)
except boto3.client("secretsmanager").exceptions.ResourceNotFoundException:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this test might be skewing the coverage from Abdel's comment above... are we sure this is still needed?

Copy link
Contributor

@kukushking kukushking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work!

@jaidisido jaidisido self-requested a review June 10, 2022 09:31
Copy link
Contributor

@jaidisido jaidisido left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to ship once CodeBuild passes

)
df = wr.oracle.read_sql_query(f'SELECT * FROM "TEST"."{table}"', oracle_con)
ensure_data_types(df, has_list=False)
# ensure_data_types(df, has_list=False)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not used, we might as well delete this line

@cnfait cnfait force-pushed the oracle-support branch 6 times, most recently from 30af625 to cfec634 Compare June 17, 2022 08:05
@cnfait cnfait merged commit 1585906 into main Jun 22, 2022
@cnfait cnfait deleted the oracle-support branch June 22, 2022 08:38
@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubCodeBuild8756EF16-VQTEJACU4zKd
  • Commit ID: 2d74a09
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants