You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by GitBox <gi...@apache.org> on 2022/07/25 17:02:47 UTC
[GitHub] [incubator-devlake] apujari-hippo opened a new pull request, #2600: [FIX]: resolve network address with connection string.
apujari-hippo opened a new pull request, #2600:
URL: https://github.com/apache/incubator-devlake/pull/2600
### ⚠️ Pre Checklist
> Please complete _ALL_ items in this checklist, and remove before submitting
- [ ] I have read through the [Contributing](https://devlake.apache.org/community/) Documentation & [PR Template](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
- [ ] This PR is using a `label` (bug, feature etc.)
- [ ] My code is has necessary documentation (if appropriate)
- [ ] I have added any relevant tests
- [ ] This section (**⚠️ Pre Checklist**) will be removed when submitting PR
# Summary
<!--
Thanks for submitting a pull request!
We appreciate you spending the time to work on these changes.
Please fill out as many sections below as possible.
-->
### Does this close any open issues?
Please mention the issues here.
### Screenshots
Include any relevant screenshots here.
### Other Information
[error] failed to initialize database, got error Error 1045: Access denied for user 'mysql'@'x.x.x.x' (using password: YES)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] apujari-hippo commented on a diff in pull request #2600: [FIX]: Fixes for k8-deploy.
Posted by GitBox <gi...@apache.org>.
apujari-hippo commented on code in PR #2600:
URL: https://github.com/apache/incubator-devlake/pull/2600#discussion_r930107707
##########
k8s-deploy.yaml:
##########
@@ -152,7 +152,7 @@ spec:
- name: config-ui
image: mericodev/config-ui:latest
ports:
- - containerPort: 4000
+ - containerPort: 30004
Review Comment:
The service target port was not working. Hence we update it to use node port.
https://github.com/apache/incubator-devlake/blob/main/k8s-deploy.yaml#L175
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] hezyin commented on a diff in pull request #2600: [FIX]: Fixes for k8-deploy.
Posted by GitBox <gi...@apache.org>.
hezyin commented on code in PR #2600:
URL: https://github.com/apache/incubator-devlake/pull/2600#discussion_r930199819
##########
k8s-deploy.yaml:
##########
@@ -206,7 +206,7 @@ spec:
- containerPort: 8080
env:
- name: DB_URL
- value: 'mysql://merico:merico@mysql:3306/lake?charset=utf8mb4&parseTime=True'
+ value: "merico:merico@tcp(mysql:3306)/lake?charset=utf8mb4&parseTime=True"
Review Comment:
Great! In that case, we don't need to update line 209, could you please revert the change? Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] apujari-hippo commented on a diff in pull request #2600: [FIX]: Fixes for k8-deploy.
Posted by GitBox <gi...@apache.org>.
apujari-hippo commented on code in PR #2600:
URL: https://github.com/apache/incubator-devlake/pull/2600#discussion_r930105468
##########
k8s-deploy.yaml:
##########
@@ -206,7 +206,7 @@ spec:
- containerPort: 8080
env:
- name: DB_URL
- value: 'mysql://merico:merico@mysql:3306/lake?charset=utf8mb4&parseTime=True'
+ value: "merico:merico@tcp(mysql:3306)/lake?charset=utf8mb4&parseTime=True"
Review Comment:
New image accepts the old DB string.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] hezyin commented on a diff in pull request #2600: [FIX]: Fixes for k8-deploy.
Posted by GitBox <gi...@apache.org>.
hezyin commented on code in PR #2600:
URL: https://github.com/apache/incubator-devlake/pull/2600#discussion_r930210348
##########
k8s-deploy.yaml:
##########
@@ -152,7 +152,7 @@ spec:
- name: config-ui
image: mericodev/config-ui:latest
ports:
- - containerPort: 4000
+ - containerPort: 30004
Review Comment:
Hi Aditya, based on [Kubernetes API spec](https://kubernetes.io/docs/reference/kubernetes-api/workload-resources/pod-v1/#ports), the `containerPort` is primarily informational.
> Exposing a port here gives the system additional information about the network connections a container uses, but is primarily informational. Not specifying a port here DOES NOT prevent that port from being exposed.
So even if we completely remove line 155, it probably won't affect our deployment. But its value should match with which port this container listens at, which is 4000. You can verify this in `config-ui`'s docker file: https://github.com/apache/incubator-devlake/blob/main/config-ui/Dockerfile#L43
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] abeizn commented on pull request #2600: [FIX]: Fixes for k8-deploy.
Posted by GitBox <gi...@apache.org>.
abeizn commented on PR #2600:
URL: https://github.com/apache/incubator-devlake/pull/2600#issuecomment-1219166410
All issues have been resolved and this pr will be closed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] abeizn closed pull request #2600: [FIX]: Fixes for k8-deploy.
Posted by GitBox <gi...@apache.org>.
abeizn closed pull request #2600: [FIX]: Fixes for k8-deploy.
URL: https://github.com/apache/incubator-devlake/pull/2600
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] apujari-hippo commented on pull request #2600: [FIX]: resolve network address with connection string.
Posted by GitBox <gi...@apache.org>.
apujari-hippo commented on PR #2600:
URL: https://github.com/apache/incubator-devlake/pull/2600#issuecomment-1194586655
@hezyin Will you please review this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] hezyin commented on a diff in pull request #2600: [FIX]: Fixes for k8-deploy.
Posted by GitBox <gi...@apache.org>.
hezyin commented on code in PR #2600:
URL: https://github.com/apache/incubator-devlake/pull/2600#discussion_r929515517
##########
k8s-deploy.yaml:
##########
@@ -12,8 +12,8 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
-
Review Comment:
Indeed a bug, thanks for catching this.
##########
k8s-deploy.yaml:
##########
@@ -152,7 +152,7 @@ spec:
- name: config-ui
image: mericodev/config-ui:latest
ports:
- - containerPort: 4000
+ - containerPort: 30004
Review Comment:
This container does listen on port 4000. And this `containerPort` value should match with `config-ui` service's `targetPort`, which is also 4000. So we don't want to change this.
##########
k8s-deploy.yaml:
##########
@@ -206,7 +206,7 @@ spec:
- containerPort: 8080
env:
- name: DB_URL
- value: 'mysql://merico:merico@mysql:3306/lake?charset=utf8mb4&parseTime=True'
+ value: "merico:merico@tcp(mysql:3306)/lake?charset=utf8mb4&parseTime=True"
Review Comment:
The error you encountered earlier was due to using an old image. The new image that we switched to (v0.12.0-beta3) during our call would take the `mysql://` connection string without issue. Do you think you could try the original connection string with the new image again and let us know if the issue persists?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org