You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/02/04 08:50:00 UTC
[GitHub] [apisix-dashboard] starsz opened a new pull request #1408: feat: support return the manager api's git hash and version
starsz opened a new pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408
Please answer these questions before submitting a pull request
- Why submit this pull request?
- [] Bugfix
- [x] New feature provided
- [ ] Improve performance
- [ ] Backport patches
- Related issues
fix #1404
### New feature or improvement
- Describe the details and related test reports.
Add `info` API to support get the backend info.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r570350392
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/version", wgin.Wraps(h.Version))
Review comment:
I'm not sure if we need to omit the prefix, but unification is appropriate.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] starsz commented on a change in pull request #1408: feat: support return the manager api's git hash and version (#1404)
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r568358840
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/apisix/manager-api/internal/handler"
Review comment:
Got it.
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
Review comment:
We can add more fields in the future if we use `/info`.
If we only want to add `hash` and `version` fields then we could use `/version`.
##########
File path: api/internal/utils/utils.go
##########
@@ -206,3 +211,14 @@ func ValueEqual(a interface{}, b interface{}) bool {
}
return bytes.Equal(aBytes, bBytes)
}
+
+// set the hash and version
+func SetHashAndVersion(hash, version string) {
Review comment:
Yes. We can.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] codecov-io commented on pull request #1408: feat: support feat info (#1404)
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#issuecomment-770681406
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1408?src=pr&el=h1) Report
> Merging [#1408](https://codecov.io/gh/apache/apisix-dashboard/pull/1408?src=pr&el=desc) (4e813bb) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/94d952f9a43a9d6507b2966b58791f789e5b74a6?el=desc) (94d952f) will **increase** coverage by `0.01%`.
> The diff coverage is `50.00%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/1408?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1408 +/- ##
==========================================
+ Coverage 45.15% 45.17% +0.01%
==========================================
Files 35 36 +1
Lines 2516 2526 +10
==========================================
+ Hits 1136 1141 +5
- Misses 1215 1220 +5
Partials 165 165
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/1408?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/utils/utils.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1408/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL3V0aWxzLmdv) | `45.88% <0.00%> (-1.68%)` | :arrow_down: |
| [api/internal/handler/tool/tool.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1408/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvdG9vbC90b29sLmdv) | `71.42% <71.42%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1408?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1408?src=pr&el=footer). Last update [94d952f...4e813bb](https://codecov.io/gh/apache/apisix-dashboard/pull/1408?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#issuecomment-773082942
FE is working on why test failed, once test fixed, we will merge.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] tokers merged pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
tokers merged pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] starsz commented on a change in pull request #1408: feat: support return the manager api's git hash and version (#1404)
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r568358840
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/apisix/manager-api/internal/handler"
Review comment:
Got it.
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
Review comment:
We can add more fields in the future if we use `/info`.
If we only want to add `hash` and `version` fields then we could use `/version`.
##########
File path: api/internal/utils/utils.go
##########
@@ -206,3 +211,14 @@ func ValueEqual(a interface{}, b interface{}) bool {
}
return bytes.Equal(aBytes, bBytes)
}
+
+// set the hash and version
+func SetHashAndVersion(hash, version string) {
Review comment:
Yes. We can.
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
+}
+
+func (h *Handler) Info(c droplet.Context) (interface{}, error) {
+ hash, version := utils.GetHashAndVersion()
+ return &InfoOutput{
+ Hash: hash,
+ Version: version,
Review comment:
Maybe the described work should be done in the FE.
In fact, it's the version of manager-api. There is no problem.
##########
File path: api/test/e2e/info_test.go
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestInfo(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ Desc: "get info",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodGet,
+ Path: "/info",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: []string{"commit_hash", "\"version\":\"master\""},
Review comment:
Because the commit_hash is always changing. So we only guarantee the key exists.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #1408: feat: support feat info (#1404)
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r567837452
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/apisix/manager-api/internal/handler"
Review comment:
style
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
Review comment:
`/info` doesn't feel reasonable…
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r568627143
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
+}
+
+func (h *Handler) Info(c droplet.Context) (interface{}, error) {
+ hash, version := utils.GetHashAndVersion()
+ return &InfoOutput{
+ Hash: hash,
+ Version: version,
Review comment:
because the manager-api and web are under the same repo `apisix-dashboard`, when users call it, we will think it's dashboard's version instead 🤔
cc @liuxiran @ShiningRush what do you think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] tokers commented on a change in pull request #1408: feat: support return the manager api's git hash and version (#1404)
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r567875136
##########
File path: api/internal/utils/utils.go
##########
@@ -206,3 +211,14 @@ func ValueEqual(a interface{}, b interface{}) bool {
}
return bytes.Equal(aBytes, bBytes)
}
+
+// set the hash and version
+func SetHashAndVersion(hash, version string) {
Review comment:
Can't understand why this function is required.
If you want to assign these two values from the go linker, just referencing them in the `-X` option.
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
Review comment:
What about `/version`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] starsz commented on a change in pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r570345216
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/version", wgin.Wraps(h.Version))
Review comment:
OK.Got it.
I will add the prefix and ignore this API's auth checking in the next 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r570317906
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/version", wgin.Wraps(h.Version))
Review comment:
@starsz @nic-chen
This will lead to 2 kinds of APIs:
1. `/apisix/admin/xxx`
2. `/version`
When we deprecate `admin-api` in V3, we will have to omit that prefix `apisix/admin` either, to have ManagerAPIs.
Now FE meets one issue: because HTTP call is handled by UmiJS, which only supports one prefix, `/apisix/admin` or `/`, then FE has to make hack codes to be compatible with those 2 kinds of APIs.
Could we add a prefix for this API? with some description about this case.
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/version", wgin.Wraps(h.Version))
Review comment:
@starsz @nic-chen
This will lead to 2 kinds of APIs:
1. `/apisix/admin/xxx`
2. `/version`
When we deprecate `admin-api` in V3, we will have to omit that prefix `apisix/admin` either, to have ManagerAPIs.
Now FE meets one issue: because HTTP call is handled by UmiJS, which only supports one prefix, `/apisix/admin` or `/`, then FE has to make hack codes to be compatible with those 2 kinds of APIs.
I know it's wired to have something like `/apisix/admin/version`, but could we add a prefix for this API first? with some description about this hacking case. In V3, we may omit that prefix from all APIs.
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/version", wgin.Wraps(h.Version))
Review comment:
@starsz @nic-chen
This will lead to 2 kinds of APIs:
1. `/apisix/admin/xxx`
2. `/version`
When we deprecate `admin-api` in V3, we will have to omit that prefix `apisix/admin` either, to have ManagerAPIs.
Now FE meets one issue: because HTTP call is handled by UmiJS, which only supports one prefix, `/apisix/admin` or `/`, then FE has to make hack codes to be compatible with those 2 kinds of APIs.
I know it's wired to have something like `/apisix/admin/version`, but could we add a prefix for this API first? with some description about this hacking case. In V3, we may omit that prefix from all APIs.
cc @LiteSun
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/version", wgin.Wraps(h.Version))
Review comment:
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] starsz commented on a change in pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r569050107
##########
File path: api/internal/utils/version.go
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package utils
+
+var (
+ gitHash string
Review comment:
IMO, It's not safe enough.
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
Review comment:
Got it. Now I think `/version` is better.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#issuecomment-772968657
@starsz try to sync with the latest codes from the master branch?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#issuecomment-773082942
FE is working on why test failed, once test fixed, we will merge.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] tokers merged pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
tokers merged pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r570317906
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/version", wgin.Wraps(h.Version))
Review comment:
@starsz @nic-chen
This will lead to 2 kinds of APIs:
1. `/apisix/admin/xxx`
2. `/version`
When we deprecate `admin-api` in V3, we will have to omit that prefix `apisix/admin` either, to have ManagerAPIs.
Now FE meets one issue: because HTTP call is handled by UmiJS, which only supports one prefix, `/apisix/admin` or `/`, then FE has to make hack codes to be compatible with those 2 kinds of APIs.
Could we add a prefix for this API? with some description about this case.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] imjoey commented on a change in pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
imjoey commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r569054930
##########
File path: api/internal/utils/version.go
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package utils
+
+var (
+ gitHash string
Review comment:
Agree with @starsz , for me, immutable variables here would be safer.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#issuecomment-772973411
fine, then please request changes from other members, once the FE test is fixed, I will let you know.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] starsz commented on a change in pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r570345216
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/version", wgin.Wraps(h.Version))
Review comment:
OK.Got it.
I will add the prefix and ignore this API's auth checking in the next 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r569094823
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
Review comment:
OK
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r570317906
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/version", wgin.Wraps(h.Version))
Review comment:
@starsz @nic-chen
This will lead to 2 kinds of APIs:
1. `/apisix/admin/xxx`
2. `/version`
When we deprecate `admin-api` in V3, we will have to omit that prefix `apisix/admin` either, to have ManagerAPIs.
Now FE meets one issue: because HTTP call is handled by UmiJS, which only supports one prefix, `/apisix/admin` or `/`, then FE has to make hack codes to be compatible with those 2 kinds of APIs.
I know it's wired to have something like `/apisix/admin/version`, but could we add a prefix for this API first? with some description about this hacking case. In V3, we may omit that prefix from all APIs.
cc @LiteSun
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #1408: feat: support return the manager api's git hash and version (#1404)
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r568373678
##########
File path: api/test/e2e/info_test.go
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestInfo(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ Desc: "get info",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodGet,
+ Path: "/info",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: []string{"commit_hash", "\"version\":\"master\""},
Review comment:
where is `commit_hash`'s value?
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
+}
+
+func (h *Handler) Info(c droplet.Context) (interface{}, error) {
+ hash, version := utils.GetHashAndVersion()
+ return &InfoOutput{
+ Hash: hash,
+ Version: version,
Review comment:
`Version` is a little confusing, it's manager-api's version actually 🤔
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
Review comment:
This endpoint is used to know about more information for the manager-api (Dashboard), API_VERSION, commit hash, etc.
@imjoey what's your opinion?
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
+}
+
+func (h *Handler) Info(c droplet.Context) (interface{}, error) {
+ hash, version := utils.GetHashAndVersion()
+ return &InfoOutput{
+ Hash: hash,
+ Version: version,
Review comment:
because the manager-api and web are under the same repo `apisix-dashboard`, when users call it, we will think it's dashboard's version instead 🤔
cc @liuxiran @ShiningRush what do you think?
##########
File path: api/test/e2e/info_test.go
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestInfo(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ Desc: "get info",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodGet,
+ Path: "/info",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: []string{"commit_hash", "\"version\":\"master\""},
Review comment:
ok
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] starsz commented on a change in pull request #1408: feat: support return the manager api's git hash and version (#1404)
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r568459066
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
+}
+
+func (h *Handler) Info(c droplet.Context) (interface{}, error) {
+ hash, version := utils.GetHashAndVersion()
+ return &InfoOutput{
+ Hash: hash,
+ Version: version,
Review comment:
Maybe the described work should be done in the FE.
In fact, it's the version of manager-api. There is no problem.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] imjoey commented on a change in pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
imjoey commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r569036244
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
Review comment:
I would prefer `/version` for now.
Just FYI, `version` indicates the details about the distribution of manager-api binary, such as `OS`, `Arch`, `Go version`, `API version` and etc. `info` indicates the status and metrics of the running manager-api binary, such as `connections`, `memory usage` and etc.
Please see the output `docker version` and `docker info` for examples. Maybe we could follow that. 😄
```
➜ apisix-dashboard git:(master) docker version
Client: Docker Engine - Community
Cloud integration: 1.0.7
Version: 20.10.2
API version: 1.41
Go version: go1.13.15
Git commit: 2291f61
Built: Mon Dec 28 16:12:42 2020
OS/Arch: darwin/amd64
Context: default
Experimental: true
Server: Docker Engine - Community
Engine:
Version: 20.10.2
API version: 1.41 (minimum version 1.12)
Go version: go1.13.15
Git commit: 8891c58
Built: Mon Dec 28 16:15:28 2020
OS/Arch: linux/amd64
Experimental: false
containerd:
Version: 1.4.3
GitCommit: 269548fa27e0089a8b8278fc4fc781d7f65a939b
runc:
Version: 1.0.0-rc92
GitCommit: ff819c7e9184c13b7c2607fe6c30ae19403a7aff
docker-init:
Version: 0.19.0
GitCommit: de40ad0
```
```
➜ apisix-dashboard git:(master) docker info
Client:
Context: default
Debug Mode: false
Plugins:
app: Docker App (Docker Inc., v0.9.1-beta3)
buildx: Build with BuildKit (Docker Inc., v0.5.1-docker)
scan: Docker Scan (Docker Inc., v0.5.0)
Server:
Containers: 9
Running: 8
Paused: 0
Stopped: 1
Images: 36
Server Version: 20.10.2
Storage Driver: overlay2
Backing Filesystem: extfs
Supports d_type: true
Native Overlay Diff: true
Logging Driver: json-file
Cgroup Driver: cgroupfs
Cgroup Version: 1
Plugins:
Volume: local
Network: bridge host ipvlan macvlan null overlay
Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
Swarm: inactive
Runtimes: io.containerd.runc.v2 io.containerd.runtime.v1.linux runc
Default Runtime: runc
Init Binary: docker-init
containerd version: 269548fa27e0089a8b8278fc4fc781d7f65a939b
runc version: ff819c7e9184c13b7c2607fe6c30ae19403a7aff
init version: de40ad0
Security Options:
seccomp
Profile: default
Kernel Version: 4.19.121-linuxkit
Operating System: Docker Desktop
OSType: linux
Architecture: x86_64
CPUs: 6
Total Memory: 1.943GiB
Name: docker-desktop
ID: E2J3:HRSM:RBMG:UQ4R:ASEK:GBXM:G5FG:KYI4:WZPM:SDQI:L7GF:EYFJ
Docker Root Dir: /var/lib/docker
Debug Mode: false
HTTP Proxy: gateway.docker.internal:3128
HTTPS Proxy: gateway.docker.internal:3129
Registry: https://index.docker.io/v1/
Labels:
Experimental: false
Insecure Registries:
127.0.0.0/8
Live Restore Enabled: false
```
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
+}
+
+func (h *Handler) Info(c droplet.Context) (interface{}, error) {
+ hash, version := utils.GetHashAndVersion()
+ return &InfoOutput{
+ Hash: hash,
+ Version: version,
Review comment:
AFAIK, manager-api and web have the same version, which is also the version of apisix-dashboard. Will the versions of manager-api and web be shown to users, respectively? 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] starsz commented on a change in pull request #1408: feat: support return the manager api's git hash and version (#1404)
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r568459679
##########
File path: api/test/e2e/info_test.go
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestInfo(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ Desc: "get info",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodGet,
+ Path: "/info",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: []string{"commit_hash", "\"version\":\"master\""},
Review comment:
Because the commit_hash is always changing. So we only guarantee the key exists.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#issuecomment-770681406
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1408?src=pr&el=h1) Report
> Merging [#1408](https://codecov.io/gh/apache/apisix-dashboard/pull/1408?src=pr&el=desc) (b22303d) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/94d952f9a43a9d6507b2966b58791f789e5b74a6?el=desc) (94d952f) will **increase** coverage by `0.05%`.
> The diff coverage is `62.50%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/1408?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #1408 +/- ##
==========================================
+ Coverage 45.15% 45.20% +0.05%
==========================================
Files 35 37 +2
Lines 2516 2524 +8
==========================================
+ Hits 1136 1141 +5
- Misses 1215 1218 +3
Partials 165 165
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/1408?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/utils/version.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1408/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL3ZlcnNpb24uZ28=) | `0.00% <0.00%> (ø)` | |
| [api/internal/handler/tool/tool.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1408/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvdG9vbC90b29sLmdv) | `71.42% <71.42%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1408?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1408?src=pr&el=footer). Last update [94d952f...b22303d](https://codecov.io/gh/apache/apisix-dashboard/pull/1408?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r568627309
##########
File path: api/test/e2e/info_test.go
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestInfo(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ Desc: "get info",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodGet,
+ Path: "/info",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: []string{"commit_hash", "\"version\":\"master\""},
Review comment:
ok
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] starsz commented on pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
starsz commented on pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#issuecomment-772971891
> @starsz try to sync with the latest codes from the master branch?
It's already the latest. Please see the CI on the master branch.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r570350392
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/version", wgin.Wraps(h.Version))
Review comment:
I'm not sure if we need to omit the prefix, but unification is appropriate.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r570317906
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/version", wgin.Wraps(h.Version))
Review comment:
@starsz @nic-chen
This will lead to 2 kinds of APIs:
1. `/apisix/admin/xxx`
2. `/version`
When we deprecate `admin-api` in V3, we will have to omit that prefix `apisix/admin` either, to have ManagerAPIs.
Now FE meets one issue: because HTTP call is handled by UmiJS, which only supports one prefix, `/apisix/admin` or `/`, then FE has to make hack codes to be compatible with those 2 kinds of APIs.
I know it's wired to have something like `/apisix/admin/version`, but could we add a prefix for this API first? with some description about this hacking case. In V3, we may omit that prefix from all APIs.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan closed pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
juzhiyuan closed pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] tokers commented on a change in pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r568585383
##########
File path: api/internal/utils/version.go
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package utils
+
+var (
+ gitHash string
Review comment:
Why not just exposing these two variables 😂
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #1408: feat: support return the manager api's git hash and version (#1404)
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r568373678
##########
File path: api/test/e2e/info_test.go
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestInfo(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ Desc: "get info",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodGet,
+ Path: "/info",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: []string{"commit_hash", "\"version\":\"master\""},
Review comment:
where is `commit_hash`'s value?
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
+}
+
+func (h *Handler) Info(c droplet.Context) (interface{}, error) {
+ hash, version := utils.GetHashAndVersion()
+ return &InfoOutput{
+ Hash: hash,
+ Version: version,
Review comment:
`Version` is a little confusing, it's manager-api's version actually 🤔
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
Review comment:
This endpoint is used to know about more information for the manager-api (Dashboard), API_VERSION, commit hash, etc.
@imjoey what's your opinion?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] tokers commented on a change in pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r568585383
##########
File path: api/internal/utils/version.go
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package utils
+
+var (
+ gitHash string
Review comment:
Why not just exposing these two variables 😂
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan closed pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
juzhiyuan closed pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #1408: feat: support return the manager api's git hash and version
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #1408:
URL: https://github.com/apache/apisix-dashboard/pull/1408#discussion_r569094561
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
+}
+
+func (h *Handler) Info(c droplet.Context) (interface{}, error) {
+ hash, version := utils.GetHashAndVersion()
+ return &InfoOutput{
+ Hash: hash,
+ Version: version,
Review comment:
Agree with you!
##########
File path: api/internal/handler/tool/tool.go
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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.
+ */
+package tool
+
+import (
+ "github.com/gin-gonic/gin"
+ "github.com/shiningrush/droplet"
+ wgin "github.com/shiningrush/droplet/wrapper/gin"
+
+ "github.com/apisix/manager-api/internal/handler"
+ "github.com/apisix/manager-api/internal/utils"
+)
+
+type Handler struct {
+}
+
+type InfoOutput struct {
+ Hash string `json:"commit_hash"`
+ Version string `json:"version"`
+}
+
+func NewHandler() (handler.RouteRegister, error) {
+ return &Handler{}, nil
+}
+
+func (h *Handler) ApplyRoute(r *gin.Engine) {
+ r.GET("/info", wgin.Wraps(h.Info))
+}
+
+func (h *Handler) Info(c droplet.Context) (interface{}, error) {
+ hash, version := utils.GetHashAndVersion()
+ return &InfoOutput{
+ Hash: hash,
+ Version: version,
Review comment:
Agree with you
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org