Skip to content

snaprepoapi: Add Region field and WithRegion ctx - #136

Merged
karencfv merged 3 commits into
elastic:masterfrom
karencfv:snaprepoapi-region-fields
Jun 11, 2020
Merged

karencfv merged 3 commits into
elastic:masterfrom
karencfv:snaprepoapi-region-fields

Conversation

@karencfv

Copy link
Copy Markdown
Contributor

Description

This patch modifies all the snaprepoapi public functions to have a
Region string required parameter which is then used to create a new
context.Context: api.WithRegion(context.Background(), params.Region)
and pass it as the API operation context.

Unit tests have been modified to include a request
assertion which ensures that the region path interpolation is used when
the /platform APIs are being called.

The repository.go, repository_params.go and repository_test.go files have
been split and reorganised into multiple files each containing a single
API context and its parameters.

Related Issues

Related: #129

How Has This Been Tested?

unit tests in ecctl

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (improves code quality but has no user-facing effect)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation

Readiness Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

@karencfv
karencfv requested a review from a team as a code owner June 11, 2020 02:19
@karencfv karencfv self-assigned this Jun 11, 2020
@karencfv

Copy link
Copy Markdown
Contributor Author

I could not for the love of god figure out why the TestSet test is failing with:

* got body {"settings":{"region":"us-east-1","bucket":"mybucket","access_key":"myaccesskey","secret_key":"mysecretkey"},"type":"s3"}
    , want {"settings":{"region":"us-east-1","bucket":"mybucket","access_key":"myaccesskey","secret_key":"mysecretkey"},"type":"s3"}

😭

@marclop, do you have any ideas on why this is happening?

@marclop

marclop commented Jun 11, 2020

Copy link
Copy Markdown
Collaborator

@karencfv ah yes, I've run across this, you need to add +"\n" to the body you're generating sadly.

@marclop marclop left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good! I thin that suggestion's going to fix the test

Comment thread pkg/api/platformapi/snaprepoapi/set_test.go Outdated
Co-authored-by: Marc Lopez Rubio <marc5.12@outlook.com>
@karencfv

Copy link
Copy Markdown
Contributor Author

@marclop thank you!

@karencfv
karencfv merged commit 9b83405 into elastic:master Jun 11, 2020
@karencfv
karencfv deleted the snaprepoapi-region-fields branch June 11, 2020 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants