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

Always respect EDGEDB_CLOUD_PROFILE when connecting to cloud #235

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

elprans
Copy link
Member

@elprans elprans commented May 9, 2023

Cloud instances, unlike local instances, do not represent local
configuration, so EDGEDB_CLOUD_PROFILE should always be respected
when connecting to such instances regardless of how compound
connection options have been specified.

@elprans elprans requested review from tailhook and fantix May 9, 2023 23:12
Copy link
Contributor

@tailhook tailhook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. But we should probably also have a shared test case to cover this issue.

@fantix
Copy link
Member

fantix commented May 13, 2023

But we should probably also have a shared test case to cover this issue.

fwiw you can revert edgedb/shared-client-testcases#22 to get at least one test case for EDGEDB_CLOUD_PROFILE I think, if we decided to proceed on with this PR.

@elprans elprans changed the title Respect EDGEDB_CLOUD_PROFILE and EDGEDB_SECRET_KEY when connecting to cloud Always respect EDGEDB_CLOUD_PROFILE when connecting to cloud Oct 30, 2023
Cloud instances, unlike local instances, do not represent local
configuration, so `EDGEDB_CLOUD_PROFILE` should always be respected
when connecting to such instances regardless of how compound
connection options have been specified.
@elprans elprans requested a review from fantix October 30, 2023 20:46
elprans added a commit to edgedb/shared-client-testcases that referenced this pull request Oct 30, 2023
This reverts commit 5f2453c.

`EDGEDB_CLOUD_PROFILE` is special.  See edgedb/edgedb-rust#235.
@elprans
Copy link
Member Author

elprans commented Oct 30, 2023

But we should probably also have a shared test case to cover this issue.

fwiw you can revert edgedb/shared-client-testcases#22 to get at least one test case for EDGEDB_CLOUD_PROFILE I think, if we decided to proceed on with this PR.

Done in edgedb/shared-client-testcases#29

elprans added a commit to edgedb/shared-client-testcases that referenced this pull request Oct 30, 2023
This reverts commit 5f2453c.

`EDGEDB_CLOUD_PROFILE` is special.  See edgedb/edgedb-rust#235.
@elprans elprans merged commit a109491 into master Oct 30, 2023
4 checks passed
elprans added a commit to edgedb/edgedb-cli that referenced this pull request Oct 30, 2023
elprans added a commit to edgedb/edgedb-cli that referenced this pull request Oct 30, 2023
elprans added a commit to edgedb/edgedb-cli that referenced this pull request Oct 30, 2023
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.

3 participants