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

Port 11015 add azure dev ops releases #1

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

shariff-6
Copy link
Owner

Description

What -

Why -

How -

Type of change

Please leave one option from the following and delete the rest:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • New Integration (non-breaking change which adds a new integration)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Non-breaking change (fix of existing functionality that will not change current behavior)
  • Documentation (added/updated documentation)

All tests should be run against the port production environment(using a testing org).

Core testing checklist

  • Integration able to create all default resources from scratch
  • Resync finishes successfully
  • Resync able to create entities
  • Resync able to update entities
  • Resync able to detect and delete entities
  • Scheduled resync able to abort existing resync and start a new one
  • Tested with at least 2 integrations from scratch
  • Tested with Kafka and Polling event listeners
  • Tested deletion of entities that don't pass the selector

Integration testing checklist

  • Integration able to create all default resources from scratch
  • Resync able to create entities
  • Resync able to update entities
  • Resync able to detect and delete entities
  • Resync finishes successfully
  • If new resource kind is added or updated in the integration, add example raw data, mapping and expected result to the examples folder in the integration directory.
  • If resource kind is updated, run the integration with the example data and check if the expected result is achieved
  • If new resource kind is added or updated, validate that live-events for that resource are working as expected
  • Docs PR link here

Preflight checklist

  • Handled rate limiting
  • Handled pagination
  • Implemented the code in async
  • Support Multi account

Screenshots

Include screenshots from your environment showing how the resources of the integration will look.

API Documentation

Provide links to the API documentation used for this integration.

Copy link

@PeyGis PeyGis left a comment

Choose a reason for hiding this comment

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

looks good overall, please work on the suggested changes. And move the PR to ocean's repo

Comment on lines +120 to +121
createdBy: .createdBy.displayName
modifiedBy: .modifiedBy.displayName
Copy link

Choose a reason for hiding this comment

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

let's use the unique name, which will be the email of the user

a sample response I saw from the docs

"modifiedBy": {
        "id": "4adb1680-0eac-6149-b5ee-fc8b4f6ca227",
        "displayName": "Chuck Reinhart",
        "uniqueName": "[email protected]",
        "url": "https://vssps.dev.azure.com/fabrikam/_apis/Identities/4adb1680-0eac-6149-b5ee-fc8b4f6ca227",
        "imageUrl": "https://dev.azure.com/fabrikam/_api/_common/identityImage?id=4adb1680-0eac-6149-b5ee-fc8b4f6ca227"
      }

port:
entity:
mappings:
identifier: .id | tostring | gsub(" "; "")
Copy link

Choose a reason for hiding this comment

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

combine the project id (.projectReference.id | gsub(" "; "")) with the release id to make it unique, so that it doesn't get overridden by other releases in other projects

Comment on lines +15 to +19
- name: vsrmUrl
description: The URL of your Visual Studio Resource Manager, Azure DevOps organization (e.g., "https://vsrm.dev.azure.com/{your-organization}"). To find your organization URL, refer to the <a target="_blank" href="https://learn.microsoft.com/en-us/azure/devops/extend/develop/work-with-urls?view=azure-devops&tabs=http">Azure DevOps documentation</a>.
required: true
type: url
sensitive: true
Copy link

Choose a reason for hiding this comment

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

since we have the organization from the main ADO url (organizationUrl), why do we need this again? is the organization id in the ADO organizationUrl different from the one in the VSRM?

@@ -25,16 +25,18 @@


class AzureDevopsClient(HTTPBaseClient):
def __init__(self, organization_url: str, personal_access_token: str) -> None:
def __init__(self, organization_url: str, vsrm_url, personal_access_token: str) -> None:
Copy link

Choose a reason for hiding this comment

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

let's remove this if the vsrm_url is the same as the ado enterprise url

Comment on lines +136 to +137
release["__projectId"] = project["id"]
release["__project"] = project
Copy link

Choose a reason for hiding this comment

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

we already get this information from the release response so we may not need this

      "projectReference": {
        "id": "d07908bc-118f-47d2-8a13-ff75601a6b1a",
        "name": "MyFirstProject"
      },

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