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

Merge DBMigrate into the core #664

Closed
neokoenig opened this Issue Aug 19, 2016 · 18 comments

Comments

7 participants
@neokoenig
Member

neokoenig commented Aug 19, 2016

NB, this is all happening on the dbmigrate branch for now.

  • Add footer link to migrations
  • Add db/sql to gitignore
  • Early proof of concept
  • Confirm whether H2 or SQLlite?
  • Refactor to script
  • Check wheels load paths + whether we can remove all these function includes
  • Write tests. Lots of tests.
  • Go through all the security implications and confirm this can be majorly locked down.
  • A config option to force entity names to lowercase or leave as defined in migrations
  • A config option to define the name of the schemainfo table
  • A config option to toggle to ability to disable writing of .sql files
  • Ability to redo a migration
  • Document

@neokoenig neokoenig added this to the 2.0.0 milestone Aug 19, 2016

@neokoenig neokoenig self-assigned this Aug 19, 2016

chapmandu added a commit that referenced this issue Aug 26, 2016

chapmandu added a commit that referenced this issue Aug 29, 2016

chapmandu added a commit that referenced this issue Aug 29, 2016

chapmandu added a commit that referenced this issue Aug 30, 2016

@chapmandu

This comment has been minimized.

Member

chapmandu commented Sep 13, 2016

I've added some configuration options that I think would be valueable

@chapmandu

This comment has been minimized.

Member

chapmandu commented Sep 13, 2016

Scriptification is complete.
I think the test suite is sufficient for public functions.. but open to suggestions.

@neokoenig

This comment has been minimized.

Member

neokoenig commented Sep 14, 2016

Still having massive problems testing Oracle locally. I think there are several major issues which aren't pushing through to the test suite due to Travis's oracle emulation via h2 (although these issues should appear in the CF10 server as that has Oracle 10g locally).

  • cfdbinfo is a mess with Oracle; as I understand it, unless you specify pattern="yourtablename" it will return all tables for all users as found in the datasource connection. If your user has DBA privs, then that can be a LOT of tables: all system tables, all views etc. So I managed to knock 32 seconds (!) off one of the tests by just specifying the tablename that it should be looking for.
  • I get massively differing test results locally compared to the test suite: Now, this may well be that I'm a) running Oracle 12c, b) set it up wrong (I have zero experience here) - but I'm not convinced the travis emulation is genuinely representing what happens "on the ground" with Oracle. I guess we've not actually tried to do anything "that" complex before now with - but now we're dropping tables etc, these issues are starting to surface. What we need is someone with (a lot more) Oracle experience to run the test suite and point out where the hell we're going wrong...

@perdjurner @chrisdpeters @timbadolato @chapmandu - any ideas?

@perdjurner

This comment has been minimized.

Contributor

perdjurner commented Sep 14, 2016

It's starting to feel like we need to drop support unless someone who
actually uses Oracle steps up to the plate...
I don't really have any other ideas.

I spent a ton of time (way back) to add / maintain Oracle support and it
was a hell I am not willing to return to :)

On Wed, Sep 14, 2016 at 11:54 AM Tom King notifications@github.com wrote:

Still having massive problems testing Oracle locally. I think there are
several major issues which aren't pushing through to the test suite due to
Travis's oracle emulation via h2 (although these issues should appear in
the CF10 server as that has Oracle 10g locally).

  • cfdbinfo is a mess with Oracle; as I understand it, unless you
    specify pattern="yourtablename" it will return all tables for all
    users as found in the datasource connection. If your user has DBA privs,
    then that can be a LOT of tables: all system tables, all views etc. So I
    managed to knock 32 seconds (!) off one of the tests by just specifying the
    tablename that it should be looking for.
  • I get massively differing test results locally compared to the
    test suite: Now, this may well be that I'm a) running Oracle 12c, b) set it
    up wrong (I have zero experience here) - but I'm not convinced the travis
    emulation is genuinely representing what happens "on the ground" with
    Oracle. I guess we've not actually tried to do anything "that" complex
    before now with - but now we're dropping tables etc, these issues are
    starting to surface. What we need is someone with (a lot more) Oracle
    experience to run the test suite and point out where the hell we're going
    wrong...

@perdjurner https://github.com/perdjurner @chrisdpeters
https://github.com/chrisdpeters @timbadolato
https://github.com/timbadolato @chapmandu https://github.com/chapmandu

  • any ideas?


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#664 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJzU3seRPopP7Fosv6YV-f_IYh1rX0pks5qp8RYgaJpZM4Joq8r
.

@neokoenig

This comment has been minimized.

Member

neokoenig commented Sep 14, 2016

I've pinged @geirman via twitter to see if he's still using Oracle

@chapmandu

This comment has been minimized.

Member

chapmandu commented Sep 14, 2016

How about the guy who submitted the last pull request with the Oracle
adapter fix.. He seems somewhat willing and definitely able..

On Wed., 14 Sep. 2016, 8:09 pm Tom King, notifications@github.com wrote:

I've pinged @geirman https://github.com/geirman via twitter to see if
he's still using Oracle


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#664 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABCr0-4J1_tC2YbamuE9FS9P3zyvid4tks5qp8e-gaJpZM4Joq8r
.

@neokoenig

This comment has been minimized.

Member

neokoenig commented Sep 14, 2016

@shutterassault ? You there?

@geirman

This comment has been minimized.

geirman commented Sep 14, 2016

@neokoenig sorry, wish I could help. I piloted wheels for a client using Oracle over a year ago, but they went sans-framework in the end

@neokoenig

This comment has been minimized.

Member

neokoenig commented Sep 14, 2016

Ah thanks anyway @geirman

@chrisdpeters

This comment has been minimized.

Contributor

chrisdpeters commented Sep 14, 2016

Oracle support as a plugin. 👍

@shutterassault

This comment has been minimized.

shutterassault commented Sep 14, 2016

I'm here. Really hope you don't drop Oracle support! It's the main db I have to use at work. We are on Oracle 11.2.0. Fairly new to Git, but can try to help. Just might need to be walked through the setup to start testing or whatever needs to be done. I submitted the last pull request because I had the same issues mentioned above about getting tables back from multiple schemas unexpectedly. What I submitted fixed that and only filters on whatever schema you specify.

@neokoenig

This comment has been minimized.

Member

neokoenig commented Sep 14, 2016

@shutterassault Hi Mike, that would be awesome - I'm happy to talk you through running the tests, if you can msg me an email address somehow, I'll put something together

@shutterassault

This comment has been minimized.

shutterassault commented Sep 14, 2016

I submitted a message on your site.

@chapmandu chapmandu self-assigned this Sep 15, 2016

@liferealized

This comment has been minimized.

Contributor

liferealized commented Dec 6, 2016

@neokoenig how is this going? Is there anything I can help with?

@neokoenig

This comment has been minimized.

Member

neokoenig commented Dec 6, 2016

hi @liferealized - everything's passing so far except Oracle. I don't like bothering
@shutterassault too much, and he's the only person I know who's actually using it :)

Working on 2.x, Oracle is becoming somewhat of a tricky thing to effectively test.

The problem is this: the Travis CI setup uses one of two methods as it can't run Oracle as a service:

  1. h2 emulation,
  2. pinging the request off to our external test server where Oracle is installed locally.

Wheels core tests therefore run under the following conditions on Travis:

  1. CF11, MSSQL <-- external test server
  2. ACF 10, Oracle, <-- external test server
  3. Lucee 4.x, MySQL <-- travis shell
  4. Lucee 4.x, PostGres <-- travis shell
  5. Lucee 4.x, H2 Oracle Emulation <-- travis shell
  6. Lucee with Wheels in a subfolder <-- travis shell

The issue is that on our (highly unpredictable) ACF/Windows/Oracle server, we're running Oracle 10g;
So tests on that box, whilst often require kicking to get through, are at least testing against a working Oracle installation.

However, the Lucee + h2 emulation just doesn't raise the same issues as Lucee + Oracle installed locally.

I've narrowed this down to drivers a bit, as I’ve finally got Docker running under windows 10:
On my local machine with Commandbox Lucee 4.x, and an Oracle 11g install running under Docker, I get 29 failures + 14 errors on the current master branch, whereas updating the Oracle driver via Lucee 5.x and using the extension Oracle driver of 11.2.0.4 (or the latest 12.x), it goes down to 1 Failure (in model.crud.finders if anyone cares).

Considering that H2 didn't/couldn't pick up those huge differences, as it was for all intents and purposes, using a different driver, this does mean there's a hole where Lucee + Oracle is concerned;

The other problem is that I can't find an effective way to return the emulation mode when h2 is in use: so I can't do "if it's Oracle, do this" in the test suite, as it thinks it's H2. This also means you can't really test for h2 specific stuff - for instance in dbmigrate, if you make getDBType() actually return h2, then any emulated tests fail; this makes things complex for using h2 with DBmigrate.

I’m tempted to jump the gun, and suggest Lucee 5.1.x as the minimum version for 2.x for start…
There’s also this - https://github.com/cbandy/travis-oracle which implies it might actually be possible to run Oracle proper under Travis, even if it is just the “express” edition…

So yeah. The combination of a not-ideal Travis setup plus my inexperience with Oracle has dragged this on a bit :)

In sum, if you fancy checking out the dbmigrate branch and having a look at Oracle + H2 generally, I would definitely buy you a pint or two.

@neokoenig

This comment has been minimized.

Member

neokoenig commented Feb 14, 2017

Now in the core via bd06c7c

Oracle tests skipped via
44f5730

@neokoenig neokoenig added the docs label Mar 15, 2017

@neokoenig neokoenig modified the milestones: 2.0.0 Beta, 2.0.0 Mar 15, 2017

@perdjurner

This comment has been minimized.

Contributor

perdjurner commented Apr 19, 2017

Moving out of Beta milestone since only docs are left to do.

@perdjurner perdjurner modified the milestones: 2.0.0, 2.0.0 Beta Apr 19, 2017

@chapmandu

This comment has been minimized.

Member

chapmandu commented Aug 7, 2017

Closing as there is a dedicated issue for docs

#744

@chapmandu chapmandu closed this Aug 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment