Skip to content
New issue

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

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

Already on GitHub? # to your account

Add credentials and token flags to all sub commands #36

Closed
wants to merge 2 commits into from

Conversation

zamai
Copy link
Contributor

@zamai zamai commented Nov 24, 2024

Hi, nice project, I used it to upload videos via bash script.

I noticed that credentials and token flags where only working inside the Auth command, and rest of the sub commands relied on the hardcoded path of the client_secret.json - which is same folder as binary execution. Which does not really work for me, so I added these flags to all subcommands (not sure if it's the correct implementation, first coding with cobra).

Not sure if you want to merge this PR as is, or just take it's implementation as starting point for next version, just thought it might be worth sharing my patch.

🙏

tiny fix, so that command is better copy paste-able.

Also, all of the shell commands have ❯  symbol  when copied, which adds a step of editing when running commands, let me know @OpenWaygate  if you allow me to remove those as well.
--credentials  → --credential
@zamai zamai changed the title [breaking changes] Add credentials and token flags to all sub commands Add credentials and token flags to all sub commands Nov 24, 2024
@OpenWaygate
Copy link
Contributor

Hi @zamai, nice catch about the document!
About these two new flags, I prefer using environment variables to modify the default value, so for these two functions:

yutu/pkg/auth/auth.go

Lines 216 to 226 in b1cc0b1

func WithCredential(cred string) Option {
return func() {
credential = cred
}
}
func WithCacheToken(token string) Option {
return func() {
cacheToken = token
}
}

We can set values like this

func WithCredential(cred string) Option {
	return func() {
		if cred != "" {
			credential = cred
			return
		}
		if cred, ok := os.LookupEnv("YUTU_CREDENTIAL"); ok {
			credential = cred
		}
	}
}

func WithCacheToken(token string) Option {
	return func() {
		// parameter > environment variable > default value
		if token != "" {
			cacheToken = token
			return
		}
		if token, ok := os.LookupEnv("YUTU_CACHE_TOKEN"); ok {
			cacheToken = token
		}
	}
}

Then when running a command with nil service

func WithService(svc *youtube.Service) Option {
return func(a *activity) {
if svc != nil {
service = svc
} else {
service = auth.NewY2BService()
}
}
}

we can pass the empty credential path and cacheToken path

func WithService(svc *youtube.Service) Option {
	return func(a *activity) {
		if svc != nil {
			service = svc
		} else {
			service = auth.NewY2BService(
				auth.WithCredential(""),
				auth.WithCacheToken(""),
			)
		}
	}
}

@zamai
Copy link
Contributor Author

zamai commented Nov 25, 2024

Sure, your project your design :)

I've already implemented flags in my fork - since that what I need in my environment. feel free to only cherrypick the docs commit.

@OpenWaygate
Copy link
Contributor

OK, I have cherry-picked your commit. Thanks again!

@OpenWaygate
Copy link
Contributor

I have updated to use environment variables, so close this pr.

@zamai zamai deleted the patch-1 branch December 3, 2024 09:21
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants