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

Add support for oracle database #1188

Closed
wants to merge 1 commit into from
Closed

Add support for oracle database #1188

wants to merge 1 commit into from

Conversation

Gared
Copy link

@Gared Gared commented Mar 20, 2013

This will add support for oracle databases. It's nearly the same code as pasted on this page: http://snipt.org/zKib2.

Tested with:
Client: Windows 32-bit, PHP 5.4.4, cakephp 2.3.1, OCI instantclient-basic-nt-11.2.0.3.0
Database server: Oracle 10g

You have to install OCI (Oracle Instant Client), enable the php extensions "php_oci8" and "php_pdo_oci" and change your database config.
Here is an example of it:

public $default = array( 'datasource' => 'Database/Oracle', 'driver' => 'oracle', 'connect' => 'oci_connect', 'persistent' => false, 'host' => 'oracle-server', 'port' => '1521', 'login' => 'login', 'password' => 'password', 'database' => '(DESCRIPTION=(ADDRESS_LIST=(ADDRESS=(PROTOCOL=TCP) (HOST=oracle-server)(PORT = 1521))) (CONNECT_DATA =(SID = mysid)(SERVER = DEDICATED)))', 'prefix' => '' );

There is one problem if you use caching: The 'database' string from the database config is used as a part of the name of the caching file. On windows I get multiple errors, because SPL has problems with several underscores somehow. I added a new value in the config file called 'alias' and replaced the part of the caching file name with it and it works, but this was a workaround only. What would you suggest is the best way to fix this issue?

@ADmad
Copy link
Member

ADmad commented Mar 20, 2013

Setting up an oracle server for testing and thus maintaining ds has been problematic for the team, which is why currently it's not available in the core. Perhaps this new ds should be added in the Datasources repo instead which is maintained with the help of the community.

@dereuromark
Copy link
Member

The code needs to be upgraded to PHP5 and especially CakePHP coding standards, by the way.

@Gared
Copy link
Author

Gared commented Mar 20, 2013

@ADmad It's sad that there is no official support for oracle. The last commit on the Datasources repo is more than a year ago. I think it don't make any sense to do a pull request to this outdated repo.
Maybe I can get it work as a plugin.

@dereuromark What do you mean with "upgrade to PHP5"? Could you please give me an example?

@dereuromark
Copy link
Member

Try running it in PHP5.3/5.4 or through code sniffer with CakePHP standard. Both will through lots of errors.
Its "public" instead of "var", also for methods. Coding standards of Cake are not respected at all, and so on.

@ADmad
Copy link
Member

ADmad commented Mar 20, 2013

  • Check the 2.0 branch of the datasources repo not master which is for 1.x. Either way it's upto the community to ensure it's well maintained 😄
  • As @dereuromark said use proper php5 visibility keywords for all methods and properties and use the official codesniffer to fix coding standard errors.
  • Also you need to add test cases.

@markstory
Copy link
Member

@ADmad perhaps the 2.0 branch of Datasources should become the master branch?

@ADmad
Copy link
Member

ADmad commented Mar 20, 2013

@markstory Yup it should, to be consistent with the main repo. The datasources repo needs some love in terms of maintenance 😄

@ceeram
Copy link
Contributor

ceeram commented Mar 20, 2013

it needs the pull-request-hack

http://felixge.de/2013/03/11/the-pull-request-hack.html

2013/3/20 ADmad notifications@github.com

@markstory https://github.com/markstory Yup it should, to be consistent
with the main repo. The datasources repo needs some love in terms of
maintenance [image: 😄]


Reply to this email directly or view it on GitHubhttps://github.com//pull/1188#issuecomment-15179902
.

@ADmad
Copy link
Member

ADmad commented Mar 20, 2013

The criteria mentioned in that blog post for awarding commit access to PR initiators seem nice.

@CauanCabral
Copy link
Contributor

👍

@ADmad
Copy link
Member

ADmad commented Aug 11, 2013

Closing this PR as per my initial comments.

@ADmad ADmad closed this Aug 11, 2013
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.

None yet

6 participants