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

Make import work for BitBucket, CodeCommit and GitLab #617

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

smoy
Copy link
Contributor

@smoy smoy commented Aug 25, 2023

What changed?

  • Generic Git Provider that that uses https protocol to check out code.

Rationale

  • Building the most generic git provider. https checkout capability. The branch does not have any fancy things like comment on pull request. Since I am still sketching out the interface.

How was it tested?

If it was manually verified, list the instructions for your reviewers to follow.

  • Unit Tests
  • Functional Tests
  • Manually Verified

Run VSCode action: "Iambic: Simulate Generic Git Provider" with GIT_PROVIDER_UNDER_TEST with either bitbucket, codecommit, or gitlab

@smoy smoy self-assigned this Aug 25, 2023
@codecov
Copy link

codecov bot commented Aug 25, 2023

Codecov Report

Patch coverage: 5.14% and project coverage change: +8.91% 🎉

Comparison is base (c768953) 73.22% compared to head (1a1ec9d) 82.14%.
Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #617      +/-   ##
==========================================
+ Coverage   73.22%   82.14%   +8.91%     
==========================================
  Files         105      107       +2     
  Lines       12588    12724     +136     
==========================================
+ Hits         9218    10452    +1234     
+ Misses       3370     2272    -1098     
Flag Coverage Δ
functional_tests 65.25% <5.14%> (?)
functional_tests_config_discovery 45.29% <0.73%> (?)
unit_tests 72.50% <5.14%> (-0.73%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
.../v0_1_0/generic_git_provider/aws_lambda_handler.py 0.00% <0.00%> (ø)
.../v0_1_0/generic_git_provider/generic_git_client.py 0.00% <0.00%> (ø)
iambic/plugins/v0_1_0/aws/cloud_formation/utils.py 21.66% <14.28%> (-1.57%) ⬇️
iambic/plugins/v0_1_0/aws/models.py 85.64% <100.00%> (+29.29%) ⬆️

... and 49 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@smoy smoy marked this pull request as ready for review August 28, 2023 18:55
iambic/config/wizard.py Outdated Show resolved Hide resolved
)

build_id = response["build"]["id"]
# FIXME explain why this is stalling for builds
Copy link
Contributor

@castrapel castrapel Aug 28, 2023

Choose a reason for hiding this comment

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

FIXME okay to leave? Comment below still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

context: this is a cicd script sitting in their build system. so I attempt to not import anything that requires another pip install.

I switch this to standard logging module in python standard library.

GENERIC_GIT_PROVIDER_SECRET_ARN = os.environ.get("GENERIC_GIT_PROVIDER_SECRET_ARN")


def run_handler(event=None, context=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This is code we'll need to maintain separately from the other Lambda? Is detect a v2 thing? Main concern is having to update lambda code in 2 places when we make any changes, but non-blocking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • detect will be a v2 thing.
  • brutal refactor requires high test coverage. high test coverage takes time. it's open for discussion.

@@ -0,0 +1,184 @@
from __future__ import annotations
Copy link
Contributor

Choose a reason for hiding this comment

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

nit / non-blocking: Does this need to be an IAMbic plugin? If so, can we try to make it a standalone plugin so we can provide a reference architecture? The mono-repo approach and the v0_1_0 in path is some existing tech debt that we'll need to work our way around sooner or later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

probably at least 2 day investment.

@smoy smoy marked this pull request as draft August 30, 2023 00:07
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