Skip to content
This repository has been archived by the owner on May 31, 2024. It is now read-only.

Added version command #41

Merged
merged 10 commits into from
Apr 2, 2021
Merged

Added version command #41

merged 10 commits into from
Apr 2, 2021

Conversation

yindia
Copy link
Contributor

@yindia yindia commented Mar 4, 2021

TL;DR

Added version command. It should return flytectl and flyteadmin version.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

Tracking Issue

flyteorg/flyte#173

Follow-up issue

@yindia yindia changed the title Added version command [WIP] Added version command Mar 4, 2021
cmd/version.go Outdated Show resolved Hide resolved
version/version.go Outdated Show resolved Hide resolved
Signed-off-by: Yuvraj <evalsocket@gmail.com>
Signed-off-by: yuvraj <evalsocket@gmail.com>
Signed-off-by: yuvraj <evalsocket@gmail.com>
@yindia yindia changed the title [WIP] Added version command Added version command Mar 24, 2021
@yindia yindia marked this pull request as ready for review March 24, 2021 07:50
@yindia yindia requested a review from kumare3 March 24, 2021 07:50
Signed-off-by: Yuvraj <yuvraj.yad001@gmail.com>
@yindia
Copy link
Contributor Author

yindia commented Mar 24, 2021

@kumare3 pr is ready you can review it.

Signed-off-by: yuvraj <evalsocket@gmail.com>
Signed-off-by: yuvraj <evalsocket@gmail.com>
@codecov
Copy link

codecov bot commented Mar 25, 2021

Codecov Report

Merging #41 (628b2a1) into master (0b5ef91) will increase coverage by 0.20%.
The diff coverage is 53.57%.

❗ Current head 628b2a1 differs from pull request most recent head fee1989. Consider uploading reports for the commit fee1989 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master      #41      +/-   ##
==========================================
+ Coverage   45.77%   45.98%   +0.20%     
==========================================
  Files          34       35       +1     
  Lines        1018     1046      +28     
==========================================
+ Hits          466      481      +15     
- Misses        482      491       +9     
- Partials       70       74       +4     
Flag Coverage Δ
unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cmd/version/version.go 53.57% <53.57%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b5ef91...fee1989. Read the comment docs.

version/version.go Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Signed-off-by: yuvraj <evalsocket@gmail.com>
@yindia
Copy link
Contributor Author

yindia commented Mar 26, 2021

@EngHabu currently we are printing output of version command in log. Do you have any suggestions?

➜  flytectl git:(version) ✗ ./bin/flytectl version
Warn: No metricsProvider set for the workqueue
INFO[0000] Using config file: [config.yaml]             
INFO[0000] Config section [adminutils] updated. No update handler registered.  src="viper.go:318"
INFO[0000] Config section [root] updated. No update handler registered.  src="viper.go:318"
INFO[0000] Config section [logger] updated. Firing updated event.  src="viper.go:320"
{"json":{"src":"viper.go:320"},"level":"info","msg":"Config section [admin] updated. Firing updated event.","ts":"2021-03-27T01:15:03+05:30"}
{"json":{"src":"viper.go:318"},"level":"info","msg":"Config section [storage] updated. No update handler registered.","ts":"2021-03-27T01:15:03+05:30"}
{"json":{"src":"client.go:30"},"level":"info","msg":"Initialized Admin client","ts":"2021-03-27T01:15:03+05:30"}
{"json":{},"level":"info","msg":"------------------------------------------------------------------------","ts":"2021-03-27T01:15:03+05:30"}
{"json":{},"level":"info","msg":"App [flytectl], Version [43f10a7], BuildSHA [43f10a7], BuildTS [2021-03-27 01:15:03.871811 +0530 IST m=+0.081155335]","ts":"2021-03-27T01:15:03+05:30"}
{"json":{},"level":"info","msg":"------------------------------------------------------------------------","ts":"2021-03-27T01:15:03+05:30"}
{"json":{},"level":"info","msg":"------------------------------------------------------------------------","ts":"2021-03-27T01:15:03+05:30"}
{"json":{},"level":"info","msg":"App [flyteadmin], Version [v0.3.7-47-gfa70a14], BuildSHA [fa70a14], BuildTS [2021-03-26 19:41:13.043553376 +0000 UTC m=+0.352678459]","ts":"2021-03-27T01:15:03+05:30"}
{"json":{},"level":"info","msg":"------------------------------------------------------------------------","ts":"2021-03-27T01:15:03+05:30"}

@yindia yindia requested a review from EngHabu March 26, 2021 19:51
Copy link
Contributor

@EngHabu EngHabu left a comment

Choose a reason for hiding this comment

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

You can also set formatter to Text if the goal is to see this in a terminal (will look nicer):
https://github.com/flyteorg/flytestdlib/blob/master/logger/config.go#L27

}

func PrintVersion(appName string, res *admin.GetVersionResponse) {
logrus.Info("------------------------------------------------------------------------")
Copy link
Contributor

Choose a reason for hiding this comment

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

plz use logger from stdlib...

Copy link
Contributor Author

@yindia yindia Apr 1, 2021

Choose a reason for hiding this comment

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

@EngHabu I can make these changes....but it's a command....we can't add its output in the log. I found that flytestdlib version package only prints the version in the log(We need to set the log level to print output). Printing log in JSON formate is not the solution in this case because it's log, not a output of a command

@yindia yindia requested a review from EngHabu April 1, 2021 20:00
Signed-off-by: yuvraj <evalsocket@gmail.com>
Signed-off-by: yuvraj <evalsocket@gmail.com>
Signed-off-by: yuvraj <evalsocket@gmail.com>
@EngHabu EngHabu merged commit e14d7b1 into flyteorg:master Apr 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants