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

PART 2:use-github-storage option validation #1270

Open
wants to merge 1 commit into
base: git-storage-option-feature
Choose a base branch
from

Conversation

begonaguereca
Copy link
Contributor

@begonaguereca begonaguereca commented Sep 25, 2024

This pull request ensures proper validation of the --use-github-storage option.

Closes: https://github.ghe.com/github/octoshift/issues/9326

  • Did you write/update appropriate tests
  • Release notes updated (if appropriate)
  • Appropriate logging output
  • Issue linked
  • Docs updated (or issue created)
  • New package licenses are added to ThirdPartyNotices.txt (if applicable)

@begonaguereca begonaguereca changed the title PART 2: GSO option validation PART 2:use-github-storage option validation Sep 25, 2024
Copy link

Unit Test Results

812 tests   812 ✅  22s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 5d36e96.

Copy link

Code Coverage

Package Line Rate Branch Rate Complexity Health
bbs2gh 79% 73% 657
ado2gh 84% 78% 627
Octoshift 87% 76% 1272
gei 79% 71% 527
Summary 84% (6843 / 8194) 75% (1544 / 2054) 3083

@begonaguereca begonaguereca changed the base branch from main to git-storage-option-feature September 27, 2024 23:33
@begonaguereca begonaguereca marked this pull request as ready for review September 27, 2024 23:33
Copy link
Collaborator

@ArinGhazarian ArinGhazarian left a comment

Choose a reason for hiding this comment

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

Mostly looks good, left a couple of comments.

Comment on lines +163 to +166
if (UseGithubStorage && AwsBucketName.HasValue())
{
throw new OctoshiftCliException("The --use-github-storage flag was provided with an AWS S3 Bucket name. Archive cannot be uploaded to both locations.");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess we also need a check for when --use-github-storage is provided with AzureStorageConnectionString

Comment on lines +71 to +73
if (AwsBucketName.HasValue() && UseGithubStorage)
{
throw new OctoshiftCliException("The --use-github-storage flag was provided with an AWS S3 Bucket name. Archive cannot be uploaded to both locations.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess we also need a check for when --use-github-storage is provided with AzureStorageConnectionString

ArchivePath = ARCHIVE_PATH,
GithubOrg = GITHUB_ORG,
GithubRepo = GITHUB_REPO,
AzureStorageConnectionString = AZURE_STORAGE_CONNECTION_STRING,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Azure storage connection string shouldn't be provided along with aws bucket name. We probably need and extra validation for when both aws and azure options are provided together but probably it'd be a scope creep for this PR. I guess we need to create a ticket to revise our validations and add missing ones.

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.

2 participants