8000 Fix the default executor host since it's changed in 1.4 by stnguyen90 · Pull Request #6098 · appwrite/appwrite · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix the default executor host since it's changed in 1.4 #6098

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

Closed
wants to merge 2 commits into from

Conversation

stnguyen90
Copy link
Contributor

What does this PR do?

Since the executor's hostname is now executor

openruntimes-executor:
container_name: openruntimes-executor
hostname: executor
<<: *x-logging

the default variable value should be updated.

Test Plan

Manual

Related PRs and Issues

None

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@stnguyen90 stnguyen90 added this to the 1.4.2 milestone Aug 31, 2023
@stnguyen90 stnguyen90 marked this pull request as ready for review August 31, 2023 20:41
@abnegate
Copy link
Member
abnegate commented Sep 1, 2023

I think we'll need to change the Dockerfile default as well

@stnguyen90
Copy link
Contributor Author

@abnegate, the env var in the Dockerfile shouldn't matter after #5971

@stnguyen90 stnguyen90 modified the milestones: 1.4.2, 1.4.3 Sep 2, 2023
@@ -748,7 +748,7 @@
'name' => '_APP_EXECUTOR_HOST',
'description' => 'The host used by Appwrite to communicate with the function executor!',
'introduction' => '0.13.0',
'default' => 'http://appwrite-executor/v1',
'default' => 'http://executor/v1',
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good, but maybe we should go back to using appwrite-executor, or have smart migration script (if value is exactly http://appwrite-executor/v1, then replace. If not, keep using their modified value)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let's leave it as executor. smarter installation command coming in: #6137

Copy link
Member

Choose a reason for hiding this comment

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

@stnguyen90 the override PR has been merged. I think now we should change it back to appwrite-executor

@stnguyen90 stnguyen90 modified the milestones: 1.4.3, 1.4.2 Sep 4, 2023
@stnguyen90 stnguyen90 requested a review from Meldiron September 4, 2023 16:43
@stnguyen90 stnguyen90 closed this Sep 5, 2023
@stnguyen90 stnguyen90 deleted the fix-default-executor-host branch September 5, 2023 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants
0