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.

Sign up for GitHub

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

Jump to bottom

[Integration][Gitlab] Bug | Gitlab integration doesnt perform Glob queries properly #1309

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

[Integration][Gitlab] Bug | Gitlab integration doesnt perform Glob queries properly #1309

wants to merge 30 commits into from

Conversation

shariff-6
Copy link
Contributor

@shariff-6 shariff-6 commented Jan 8, 2025 •
edited
Loading

Description

What

  • Improved glob pattern matching to align with expected behavior for mapping.
  • Added extensive unit tests to cover different scenarios.

Why

  • To ensure proper handling of glob patterns.
  • To increase test coverage .

How

  • Updated the _match function to handle "**" more effectively, allowing for empty string_parts.

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.

@shariff-6 shariff-6 requested a review from a team as a code owner January 8, 2025 15:16
@github-actions github-actions bot added the size/M label Jan 8, 2025
@shariff-6 shariff-6 changed the title [Bug][Gitlab] | Gitlab integration doesnt perform Glob queries properly [Integration][Gitlab] Bug | Gitlab integration doesnt perform Glob queries properly Jan 8, 2025
@shariff-6
Updated changelog and version
f7e9ccf
@shariff-6
Removes redundant check
40b52dd
@shariff-6
refactor(gitlab): simplify file search to use GitLab's native path ma…
2a92879 
…tching

Removes custom pattern matching in search_files_in_project in favor of GitLab's native path: search capability. This change:
- Removes redundant pattern filtering using does_pattern_apply
- Uses GitLab's search API path parameter directly
- Updates tests to reflect GitLab's actual search behavior
@shariff-6
Merge branch 'main' of https://github.com/port-labs/ocean into PORT-1…
78819e1 
…1280-Bug-Gitlab-integration-doesnt-perform-Glob-queries-properly
@shariff-6
Update versions
3a9b576
@shariff-6
refactor(gitlab): simplify glob pattern matching using glob2.fnmatch
0c94537 
- Replace custom pattern matching implementation with glob2.fnmatch
@github-actions github-actions bot added size/L and removed size/M labels Jan 16, 2025
@shariff-6
Fixes lint
f3ee1ff
@shariff-6
removes unused imports
7194217
@shariff-6
Returns generate_ref function
b105ea3
@shariff-6
refactor: simplify GitLab search with braceexpand
575dc0a 
Refactored GitLab file search to:
- Add braceexpand pattern conversion
- Update search function to use converted patterns
- Consolidate search tests into parameterized test
- Remove redundant test cases
@shariff-6
Merge branch 'main' of https://github.com/port-labs/ocean into PORT-1…
b347e05 
…1280-Bug-Gitlab-integration-doesnt-perform-Glob-queries-properly
@shariff-6
Updates version
95cfa50
@shariff-6
Fixes mypy
f10bd2c
@shariff-6
Fixes type
46b46c0
@shariff-6
Perform file search for paths concurrently
c0aa322
@shariff-6
Fixes lint
e5cdfa0
@shariff-6
Replaces undefined variable
6c2c2e1
@shariff-6
Merge branch 'main' of https://github.com/port-labs/ocean into PORT-1…
56d1cfe 
…1280-Bug-Gitlab-integration-doesnt-perform-Glob-queries-properly
@shariff-6
Updates version and changelog
e212604
@shariff-6
Fixes lock file
4bc4be8
matan84
matan84 requested changes Jan 22, 2025
View reviewed changes
Copy link
Member

@matan84 matan84 left a comment

Choose a reason for hiding this comment

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

Left a few comments

integrations/gitlab/CHANGELOG.md Outdated Show resolved Hide resolved
integrations/gitlab/gitlab_integration/gitlab_service.py Show resolved Hide resolved
integrations/gitlab/gitlab_integration/gitlab_service.py Outdated Show resolved Hide resolved
integrations/gitlab/gitlab_integration/gitlab_service.py
async for files in AsyncFetcher.fetch_batch(
project.search,
scope="blobs",
search=f"filename:{file_pattern}",
search=f"path:{pattern}",
Copy link
Member

Choose a reason for hiding this comment

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

Why does this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

filename is being deprecated in the gitlab search api, see
https://docs.gitlab.com/ee/api/search.html#scope-blobs-2

integrations/gitlab/tests/gitlab_integration/test_gitlab_service.py Outdated Show resolved Hide resolved
@shariff-6
- Simplified the pattern conversion function for better readability.
56c2ac6 
- Moved pattern processing to a new method for cleaner code.
- Improved logging to show more useful information.
- Updated tests to simplify and organize file search scenarios.
@shariff-6
Merge branch 'main' of https://github.com/port-labs/ocean into PORT-1…
173d94e 
…1280-Bug-Gitlab-integration-doesnt-perform-Glob-queries-properly
@shariff-6 shariff-6 requested a review from matan84 January 22, 2025 14:49
@mk-armah mk-armah requested review from PeyGis, Tankilevitch and a team January 26, 2025 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matan84 matan84 Awaiting requested review from matan84

@PeyGis PeyGis Awaiting requested review from PeyGis

@Tankilevitch Tankilevitch Awaiting requested review from Tankilevitch

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@shariff-6 @matan84