Skip to content

feat(kafka topic list): add flags for pagination #810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rkpattnaik780 merged 8 commits into from
Jul 12, 2021
Merged

feat(kafka topic list): add flags for pagination #810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
af47146
feat(kafka topic list): add flags for pagination
rkpattnaik780 Jul 7, 2021
a1c1b67
fix: address review comments
rkpattnaik780 Jul 8, 2021
74269bd
docs(kafka topic list): update docs
rkpattnaik780 Jul 8, 2021
462ab64
fix: remove validator methods for page and size
rkpattnaik780 Jul 8, 2021
01f7350
fix: failing build due to unused vars
rkpattnaik780 Jul 8, 2021
20a2494
fix: use build variables
rkpattnaik780 Jul 9, 2021
f63d62b
fix: typo in makefile
rkpattnaik780 Jul 9, 2021
5d805b2
docs: regenerate docs
rkpattnaik780 Jul 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,19 @@ REPOSITORY_NAME ?= "app-services-cli"
TERMS_REVIEW_EVENT_CODE ?= "onlineService"
TERMS_REVIEW_SITE_CODE ?= "ocm"

# see pkg/cmdutil/constants.go
DEFAULT_PAGE_NUMBER ?= 1
DEFAULT_PAGE_SIZE ?= 10

GO_LDFLAGS := -X github.com/redhat-developer/app-services-cli/internal/build.Version=$(RHOAS_VERSION) $(GO_LDFLAGS)
GO_LDFLAGS := -X github.com/redhat-developer/app-services-cli/internal/build.RepositoryOwner=$(REPOSITORY_OWNER) $(GO_LDFLAGS)
GO_LDFLAGS := -X github.com/redhat-developer/app-services-cli/internal/build.RepositoryName=$(REPOSITORY_NAME) $(GO_LDFLAGS)
GO_LDFLAGS := -X github.com/redhat-developer/app-services-cli/internal/build.TermsReviewEventCode=$(TERMS_REVIEW_EVENT_CODE) $(GO_LDFLAGS)
GO_LDFLAGS := -X github.com/redhat-developer/app-services-cli/internal/build.TermsReviewSiteCode=$(TERMS_REVIEW_SITE_CODE) $(GO_LDFLAGS)

GO_LDFLAGS := -X github.com/redhat-developer/app-services-cli/pkg/cmdutil.DefaultPageSize=$(DEFAULT_PAGE_SIZE) $(GO_LDFLAGS)
GO_LDFLAGS := -X github.com/redhat-developer/app-services-cli/pkg/cmdutil.DefaultPageNumber=$(DEFAULT_PAGE_NUMBER) $(GO_LDFLAGS)

BUILDFLAGS :=

ifdef DEBUG
Expand Down
2 changes: 2 additions & 0 deletions docs/commands/rhoas_kafka_topic_list.adoc
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions pkg/cmd/kafka/topic/create/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func NewCreateTopicCommand(f *factory.Factory) *cobra.Command {

if !opts.interactive {

validator := &topicutil.Validator{
validator := topicutil.Validator{
Localizer: opts.localizer,
}

Expand Down Expand Up @@ -222,7 +222,7 @@ func runInteractivePrompt(opts *Options) (err error) {
return err
}

validator := &topicutil.Validator{
validator := topicutil.Validator{
Localizer: opts.localizer,
InstanceID: opts.kafkaID,
Connection: opts.Connection,
Expand Down
30 changes: 25 additions & 5 deletions pkg/cmd/kafka/topic/list/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import (
"context"
"encoding/json"
"errors"
"net/http"

"github.com/redhat-developer/app-services-cli/pkg/cmdutil"
topicutil "github.com/redhat-developer/app-services-cli/pkg/kafka/topic"
"github.com/redhat-developer/app-services-cli/pkg/localize"

Expand Down Expand Up @@ -35,6 +37,8 @@ type Options struct {
kafkaID string
output string
search string
page int32
size int32
}

type topicRow struct {
Expand Down Expand Up @@ -67,8 +71,16 @@ func NewListTopicCommand(f *factory.Factory) *cobra.Command {
}
}

if opts.page < 1 {
return errors.New(opts.localizer.MustLocalize("kafka.topic.list.validation.page.error.invalid.minValue", localize.NewEntry("Page", opts.page)))
}

if opts.size < 1 {
return errors.New(opts.localizer.MustLocalize("kafka.topic.list.validation.size.error.invalid.minValue", localize.NewEntry("Size", opts.size)))
}

if opts.search != "" {
validator := &topicutil.Validator{
validator := topicutil.Validator{
Localizer: opts.localizer,
}
if err := validator.ValidateSearchInput(opts.search); err != nil {
Expand All @@ -93,6 +105,8 @@ func NewListTopicCommand(f *factory.Factory) *cobra.Command {

cmd.Flags().StringVarP(&opts.output, "output", "o", "", opts.localizer.MustLocalize("kafka.topic.list.flag.output.description"))
cmd.Flags().StringVarP(&opts.search, "search", "", "", opts.localizer.MustLocalize("kafka.topic.list.flag.search.description"))
cmd.Flags().Int32VarP(&opts.page, "page", "", int32(cmdutil.DefaultPageNumber), opts.localizer.MustLocalize("kafka.topic.list.flag.page.description"))
cmd.Flags().Int32VarP(&opts.size, "size", "", int32(cmdutil.DefaultPageSize), opts.localizer.MustLocalize("kafka.topic.list.flag.size.description"))

flagutil.EnableOutputFlagCompletion(cmd)

Expand Down Expand Up @@ -122,6 +136,10 @@ func runCmd(opts *Options) error {
a = a.Filter(opts.search)
}

a = a.Size(opts.size)

a = a.Page(opts.page)

topicData, httpRes, err := a.Execute()
if err != nil {
if httpRes == nil {
Expand All @@ -131,19 +149,21 @@ func runCmd(opts *Options) error {
operationTemplatePair := localize.NewEntry("Operation", "list")

switch httpRes.StatusCode {
case 401:
case http.StatusUnauthorized:
return errors.New(opts.localizer.MustLocalize("kafka.topic.list.error.unauthorized", operationTemplatePair))
case 403:
case http.StatusForbidden:
return errors.New(opts.localizer.MustLocalize("kafka.topic.list.error.forbidden", operationTemplatePair))
case 500:
case http.StatusInternalServerError:
return errors.New(opts.localizer.MustLocalize("kafka.topic.common.error.internalServerError"))
case 503:
case http.StatusServiceUnavailable:
return errors.New(opts.localizer.MustLocalize("kafka.topic.common.error.unableToConnectToKafka", localize.NewEntry("Name", kafkaInstance.GetName())))
default:
return err
}
}

defer httpRes.Body.Close()

if topicData.GetTotal() == 0 && opts.output == "" {
logger.Info(opts.localizer.MustLocalize("kafka.topic.list.log.info.noTopics", localize.NewEntry("InstanceName", kafkaInstance.GetName())))

Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/kafka/topic/update/update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func NewUpdateTopicCommand(f *factory.Factory) *cobra.Command {
return cmdutil.FilterValidTopicNameArgs(f, toComplete)
},
RunE: func(cmd *cobra.Command, args []string) (err error) {
validator := &topicutil.Validator{
validator := topicutil.Validator{
Localizer: opts.localizer,
}

Expand Down Expand Up @@ -338,7 +338,7 @@ func runInteractivePrompt(opts *Options) (err error) {
return err
}

validator := &topicutil.Validator{
validator := topicutil.Validator{
Localizer: opts.localizer,
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/cmdutil/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,10 @@ package cmdutil
const (
// The default indentation to use when printing data to stdout
DefaultJSONIndent = " "

// DefaultPageSize is the default number of items per page when using list commands
DefaultPageSize = 10
Copy link
Contributor

@craicoverflow craicoverflow Jul 9, 2021

Choose a reason for hiding this comment

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

In a follow up PR, could you apply these wherever there is pagination?


// DefaultPageNumber is the default page number when using list commands
DefaultPageNumber = 1
Comment on lines +8 to +11
Copy link
Contributor

@craicoverflow craicoverflow Jul 9, 2021

Choose a reason for hiding this comment

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

I think these needs to to be vars to be configurable at build-time.

Copy link
Contributor

@craicoverflow craicoverflow Jul 9, 2021

Choose a reason for hiding this comment

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

Build variables can only be a string 🤔 Leave it like this for now and we can figure out how to configure it later (leave a TODO)

Copy link
Contributor Author

@rkpattnaik780 rkpattnaik780 Jul 12, 2021

Choose a reason for hiding this comment

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

Checked now it doesnt work as expected! It needs to be addressed in a follow-up.

)
14 changes: 14 additions & 0 deletions pkg/localize/locales/en/cmd/kafka_topic_list.en.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,20 @@ one = 'No topics found in Kafka instance "{{.InstanceName}}"'
description = 'Description for the --search flag'
one = 'Text search to filter the Kafka topics by name'

[kafka.topic.list.flag.page.description]
description = 'Description for the --page flag'
one = 'Current page number for list of topics.'

[kafka.topic.list.flag.size.description]
description = 'Description for the --size flag'
one = 'Maximum number of items to be returned per page.'

[kafka.topic.list.validation.page.error.invalid.minValue]
one = 'invalid page number {{.Page}}, minimum value is -1'

[kafka.topic.list.validation.size.error.invalid.minValue]
one = 'invalid value for size {{.Size}}, minimum value is -1'

[kafka.topic.list.log.debug.filteringTopicList]
description = 'Debug message when filtering the list of Kafka topic'
one = 'Filtering Kafka topics with the query "{{.Search}}"'
Expand Down