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

Separate new and connect #570

Open
andyundso opened this issue Dec 17, 2024 · 2 comments
Open

Separate new and connect #570

andyundso opened this issue Dec 17, 2024 · 2 comments

Comments

@andyundso
Copy link
Member

(this is just an idea for now)

When creating a new instance of a TinyTds::Client, it immediately tries to connect to the database. It might be a better pattern to separate the connect part to a new method that needs to be called separately.

For one, you could call escape on the Client without requiring a live connection to the server.

Second, we could investigate to hand off the GVL when calling dbopen and dbuse. Both of these functions communicate with the server. In case the connection is slow / bad, it blocks the current Ruby thread. I personally would feel more comfortable handing off the GVL within a method instead of when we initialize a new object in Ruby space.

Technically, the connect method on the client is already public. It would be more about removing it from initialize.

@aharpervc
Copy link
Contributor

For one, you could call escape on the Client without requiring a live connection to the server.

Yup, completely agree. This has always been a massive pain point for me. Another approach would be to make escape a class method, so it doesn't even need an instance to be called.

Additionally... does it even need to be in C space? This seems extremely peculiar and not nearly as robust as you'd hope/assume. What if escape was just pure Ruby?

opt_escape_regex = rb_funcall(rb_cRegexp, intern_new, 1, rb_str_new2("\\\'"));
opt_escape_dblquote = rb_str_new2("''");
rb_global_variable(&opt_escape_regex);
rb_global_variable(&opt_escape_dblquote);

@andyundso
Copy link
Member Author

I was wondering the same - I think it can be pure Ruby since it does not call anything that requires it to be in C space. And class method would also make sense. It could even be it's own separate small class (TinyTds::Encoder?)

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

No branches or pull requests

2 participants