Skip to content
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.

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

Use shared cache implementation #13

Merged
merged 3 commits into from Sep 22, 2022
Merged

Use shared cache implementation #13

merged 3 commits into from Sep 22, 2022

Conversation

ofalvai
Copy link
Contributor

@ofalvai ofalvai commented Sep 21, 2022

Checklist

  • I've read and followed the Contribution Guidelines
  • step.yml and README.md is updated with the changes (if needed)

Version

Requires a MAJOR/MINOR/PATCH version update

Context

Using shared cache implementation from go-steputils. There is no functionality change in this PR.

Changes

Remove code that had been moved to go-steputils and call that code instead.

Investigation details

Decisions

@@ -171,7 +154,7 @@ workflows:
--data "client_id=abcs-steps" \
--data "client_secret=$CACHE_API_CLIENT_SECRET" \
--data "grant_type=urn:ietf:params:oauth:grant-type:uma-ticket" \
--data "claim_token=eyJhcHBfaWQiOlsic3RlcC1yZXN0b3JlLWNhY2hlLWUyZS10ZXN0cyJdLCAiaXNfY3JlZGl0X2Jhc2VkIjpbInRydWUiXSwgImlzX3ByaXZhdGVfY2xvdWQiOlsidHJ1ZSJdfQ==" \
--data "claim_token=eyJhcHBfaWQiOlsic3RlcC1zYXZlLWNhY2hlLWUyZS10ZXN0cyJdLCAiYWNjZXNzX2dyYW50ZWQiOlsidHJ1ZSJdfQ==" \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an unrelated change, but a backend-side change broke E2E tests. The base64 encoded claim token need to have different fields now.

@ofalvai ofalvai marked this pull request as ready for review September 21, 2022 12:50
@@ -32,14 +40,14 @@ func Upload(params UploadParams, logger log.Logger) error {
}
logger.Debugf("Upload ID: %s", resp.ID)

logger.Println()
logger.Debugf("")
Copy link

Choose a reason for hiding this comment

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

I guess this is intentional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, because debug-level logs are only printed if the verbose input is set to true. We don't want to always log an empty line, just when debug logging is turned on

return "", fmt.Errorf("commas are not allowed in key")
}

if len(key) > maxKeyLength {
Copy link

Choose a reason for hiding this comment

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

Don't we want to log this to the console when the key is truncated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, I'll create a follow-up PR in go-steputils

@ofalvai ofalvai merged commit e3b1aec into main Sep 22, 2022
@ofalvai ofalvai deleted the PLANG-2418-use-shared-impl branch September 22, 2022 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants