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

Revert "Merge pull request #5540 from dojutsu-user/env-var-json" #5618

Closed
wants to merge 1 commit into from

Conversation

ericholscher
Copy link
Member

This reverts commit c5e705e, reversing
changes made to a992ad1.

This reverts commit c5e705e, reversing
changes made to a992ad1.
@ericholscher ericholscher added the PR: hotfix Pull request applied as hotfix to release label Apr 22, 2019
@humitos
Copy link
Member

humitos commented Apr 22, 2019

We had to revert this commit because this line

https://github.com/rtfd/readthedocs.org/pull/5540/files#diff-31f8391d8643c47c6caedf53930c0b7eR239

is hitting the DB and builders don't have access to the database in production.

We should use get this value from version.project.environment_variables instead (this value comes from the API call that the builder does).

@dojutsu-user
Copy link
Member

dojutsu-user commented Apr 23, 2019

@humitos
I did some research and we did have version.project.environment_variables as a dictionary. Thank you for the information.
I have made another PR to fix this. #5622

Can we somehow catch these errors in development environment while testing?

@humitos
Copy link
Member

humitos commented Apr 25, 2019

Can we somehow catch these errors in development environment while testing?

We have been thinking on a way to reproduce that architecture we have in production, but we don't have a real solution yet :(

@humitos humitos closed this May 20, 2019
@humitos humitos deleted the revert-env-var-json branch May 20, 2019 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: hotfix Pull request applied as hotfix to release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants