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

correction to use with sails 1.0 #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dcunited001
Copy link

Can this be safely merged in? (again, I'm a sails noob, but I'd like to use a consistent remote/head for the project.)

@ndarilek
Copy link

Would love to see this merged if it can happen as well. Also new to Sails but need this for a legacy project. Anyone more knowledgable than me care to comment?

Thanks!

@ndarilek
Copy link

I just merged #6 and #5 here. We need this for a project, so if anyone could test and make sure it works, that'd be great. I'd rather not have to maintain this as I don't know what's involved, but at least now there's a place to get code that might work on Sails >1.

@misterGF
Copy link
Owner

I believe this would break backwards compatibility. Haven't had the time yet to look into it.

Can you resubmit this PR to the v1 branch? We can use that for V1 users and keep master for the 0.x users (for now).

@ndarilek
Copy link

This isn't my PR, but I don't see a v1 branch. Is it called something else?

In any case, I'm having some issues getting this to work. The stacktrace I'm currently getting isn't valid for any committed code since I'm inserting logging and my line numbers are off, but basically it's:

TypeError: Cannot read property 'definition' of undefined
    at Object.getPrimaryKey (/host/node_modules/sails-mssqlserver/lib/adapter.js:545:57)

I think that's the right line number, I'm adjusting it for the logging I've added. In my code it reads:

        connections[connection].collections[collection].definition

Anyhow, I'm about to wrap up for the week here, but will continue looking into this next week. Hopefully it's just a quick fix. Just thought I'd put it out there in case anyone had any thoughts.

@ndarilek
Copy link

FWIW, the fix isn't a quick one. Various functions, parameter names, shapes, etc. have changed post Sails 1.0.

I'm very slowly working on bringing this up to spec at https://github.com/ndarilek/sails-mssqlserver/tree/v1/. I don't really know mssql, am using it in anger, and just want to get this working. As such, some of my code is a bit ugly. Help very much appreciated, especially as Sails doesn't really document their adapter spec.

Thanks.

@HeywoodKing
Copy link

hello,I'm a sails noob, I want to connect the sqlserver database in a sail 1.0,For help

@ndarilek
Copy link

ndarilek commented Sep 26, 2018 via email

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.

5 participants