Skip to content
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

feat/oracle-db-connector-with-refactoring-db-connectors #606

Merged
merged 46 commits into from
Dec 10, 2024

Conversation

kirtimanmishrazipstack
Copy link
Contributor

@kirtimanmishrazipstack kirtimanmishrazipstack commented Aug 21, 2024

What

  • Oracle db connector
  • Code refactoring of all db connectors

Why

  • Oracle db connector needs implementation
  • Code refactoring is need for for making the code robust and simpler to understand.

How

  • Changes in respective connectors + database_utils.py

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

  • This PR refactirs db connectors part, all connectors have been tested locally. So all connectors needs to be tested on staging
  • This PR adds a new db connector oracle-db

Database Migrations

  • N/A

Env Config

  • N/A

Relevant Docs

Related Issues or PRs

Dependencies Versions

Notes on Testing

  • All connectors tested on workflow

Screenshots

Screenshot from 2024-08-29 10-20-32
Screenshot from 2024-08-29 10-16-14
Screenshot from 2024-08-29 10-16-05

Checklist

I have read and understood the Contribution Guidelines.

@kirtimanmishrazipstack kirtimanmishrazipstack marked this pull request as draft August 21, 2024 16:21
@kirtimanmishrazipstack kirtimanmishrazipstack requested review from a team September 2, 2024 04:44
Copy link
Contributor

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

Choose a reason for hiding this comment

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

LGTM - left some minor comments

Copy link
Contributor

@muhammad-ali-e muhammad-ali-e 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, Added Some minor comments. also please check the pre-commit issues .

@kirtimanmishrazipstack kirtimanmishrazipstack changed the title feat/oracle-db-connector feat/oracle-db-connector-with-refactoring-db-connector Dec 10, 2024
@kirtimanmishrazipstack kirtimanmishrazipstack changed the title feat/oracle-db-connector-with-refactoring-db-connector feat/oracle-db-connector-with-refactoring-db-connectors Dec 10, 2024
Copy link
Contributor

filepath function $$\textcolor{#23d18b}{\tt{passed}}$$ SUBTOTAL
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_logs}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_cleanup}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_cleanup\_skip}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_client\_init}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_image\_exists}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_image}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_container\_run\_config}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_container\_run\_config\_without\_mount}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_run\_container}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{TOTAL}}$$ $$\textcolor{#23d18b}{\tt{9}}$$ $$\textcolor{#23d18b}{\tt{9}}$$

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
12.8% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@hari-kuriakose
Copy link
Contributor

Duplication will be handled once Django apps v1 is removed later.

@hari-kuriakose hari-kuriakose merged commit fabc6bd into main Dec 10, 2024
6 of 7 checks passed
@hari-kuriakose hari-kuriakose deleted the UN-1582-ABS-CBN-Add-Oracle-connector branch December 10, 2024 05:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants