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

Added support of Oracle #258

Closed
wants to merge 0 commits into from
Closed

Conversation

pikhovkin
Copy link

Added support of Oracle

@avelis
Copy link
Collaborator

avelis commented Jan 12, 2018

@pikhovkin I appreciate the effort to add Oracle support for django-silk. I see 2 PR's open. Is there a reason for that? Should those be discarded and this one be the only one for consideration to merge in?

@pikhovkin
Copy link
Author

You can remove 2 others PR's.

@albertyw
Copy link
Member

It seems the tests are failing. Can you fix them?

Also, does travis support oracle as a DB? If it does, it would be nice to add to .travis.yml to ensure compatibility.

@pikhovkin
Copy link
Author

pikhovkin commented Jan 16, 2018

We must get rid of the UUID fields, because django Oracle backend interprets them in SQL as a number, because of this an error occurs.
So I added the migrations in which I deleted these fields.
But tests check these UUID fields and raise error.

@pikhovkin
Copy link
Author

pikhovkin commented Jan 16, 2018

@albertyw I have not worked yet with travis or other like system.
https://github.com/cbandy/travis-oracle

@pikhovkin
Copy link
Author

@albertyw, please, look erroneous tests https://travis-ci.org/jazzband/django-silk/builds/329821906

@Archmonger
Copy link
Contributor

@pikhovkin This PR needs a rebase to the latest django-silk:master, can you work this?

@codecov
Copy link

codecov bot commented Sep 6, 2021

Codecov Report

Merging #258 (669dcda) into master (ef11915) will decrease coverage by 51.41%.
The diff coverage is 40.42%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #258       +/-   ##
===========================================
- Coverage   82.87%   31.46%   -51.42%     
===========================================
  Files          50       52        +2     
  Lines        2055     2082       +27     
===========================================
- Hits         1703      655     -1048     
- Misses        352     1427     +1075     
Impacted Files Coverage Δ
silk/model_factory.py 15.72% <ø> (-66.82%) ⬇️
silk/views/requests.py 44.57% <ø> (-48.20%) ⬇️
silk/views/summary.py 34.84% <0.00%> (-57.09%) ⬇️
silk/migrations/0007_add_support_oracle.py 54.54% <54.54%> (ø)
silk/migrations/0008_fix_request_prof_file_null.py 100.00% <100.00%> (ø)
silk/models.py 50.44% <100.00%> (-36.46%) ⬇️
project/project/urls.py 0.00% <0.00%> (-100.00%) ⬇️
silk/utils/data_deletion.py 0.00% <0.00%> (-100.00%) ⬇️
silk/templatetags/silk_nav.py 0.00% <0.00%> (-100.00%) ⬇️
silk/views/sql.py 0.00% <0.00%> (-96.56%) ⬇️
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef11915...669dcda. Read the comment docs.

Copy link
Contributor

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

we moved to github actions, the tests needed to pass

@SebCorbin
Copy link
Contributor

Well, I messed up trying to work on the fork, I have opened another PR to continue discussion in #619

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.

6 participants