Skip to content

Avoid early connection#39

Merged
rpbaltazar merged 1 commit intorails-on-services:developmentfrom
bukhr:feature/avoid-early-connection
May 18, 2020
Merged

Avoid early connection#39
rpbaltazar merged 1 commit intorails-on-services:developmentfrom
bukhr:feature/avoid-early-connection

Conversation

@fsateler
Copy link
Copy Markdown
Contributor

@fsateler fsateler commented Apr 11, 2020

Fixes influitive/apartment#607 in a more general way than #16

@rpbaltazar
Copy link
Copy Markdown
Contributor

rpbaltazar commented Apr 13, 2020

The code seems to make sense to me, but it breaks all the specs. can you please take a look a it?

@fsateler
Copy link
Copy Markdown
Contributor Author

fsateler commented Apr 13, 2020

I could not understand the failure completely, but it appears to be related to recursive calls to connection. I have solved the problem by ensuring we only init once per Reload.

@rpbaltazar
Copy link
Copy Markdown
Contributor

rpbaltazar commented May 14, 2020

I'm opening a new PR #50 with the changes because there has been no reply. I'll look at it and see if i can fix the issue.

@fsateler
Copy link
Copy Markdown
Contributor Author

fsateler commented May 14, 2020

Sorry, I dropped the ball. The problem seems to be that init does not invoke process_excluded_models. Maybe it should? I have added a switch to the current tenant, lets see if the tests passes. Not sure if the fix makes sense.

@fsateler
Avoid getting a connection to the database before it's necessary
256de6f 
We init the first time we request a connection. On code reloads, we
"forget" about our init state and we call it again.

After init, ensure the search path is set properly
@fsateler
Copy link
Copy Markdown
Contributor Author

fsateler commented May 14, 2020

OK, so it seems the problem was instead I was relying on an instance variable in ActiveRecord::Base. Switching that to a variable in Apartment::Tenant fixed most of the tests. To fix the remaining ones, I had to move init to the adapters so that the postgres adapter can set the schema search path. I don't know why that is needed but it doesn't seem wrong either.

@fsateler
Copy link
Copy Markdown
Contributor Author

fsateler commented May 14, 2020

Rubocop failure appears to be configuration-related.

@rpbaltazar
Copy link
Copy Markdown
Contributor

rpbaltazar commented May 18, 2020

Thank you. Will get it merged and will start testing against our test suite, as well as out staging env

@rpbaltazar rpbaltazar merged commit 26fc746 into rails-on-services:development May 18, 2020
@rpbaltazar rpbaltazar mentioned this pull request May 18, 2020
Closed
@fsateler fsateler deleted the feature/avoid-early-connection branch May 19, 2020 14:34
@rpbaltazar rpbaltazar mentioned this pull request Jun 1, 2020
Merged
rpbaltazar added a commit that referenced this pull request Jun 2, 2020
@rpbaltazar
Merge pull request #59 from rails-on-services/development
1dd198d 
Prepare Release - 2.6.1

# Enhancements

- N/a

# Bugfixes

- [Resolves influitive#607] Avoid early connection
  - #39
  - #53
  - #51
- [Resolves #52] Rake db:setup tries to seed non existent tenant - #54
- [Resolves #56] DB rollback uses second last migration - #57

# Chores

- N/a
@fsateler fsateler mentioned this pull request Oct 6, 2020
Closed
@lunks lunks mentioned this pull request Oct 26, 2020
Merged
@rpbaltazar rpbaltazar mentioned this pull request Dec 16, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rpbaltazar rpbaltazar rpbaltazar approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Establishes connection during application boot

2 participants

@fsateler @rpbaltazar