Skip to content

Commit dbff904

Browse files
[Integration][GItlab] Revert changes made in attempt to resolve race conditions (#1343)
# Description What - Revert changes made to the integration in 0.2.1 Why - 0.2.1 made significant changes to real time events handling.. I'm releasing this in attempt to role out chances that the issue of merge request events not syncing was sparked by 0.2.1 and if it actually is, then we'd have to trace the background of the problem 0.2.1 sought to solve and re-solve. How - Restore state of real time events prior to 0.2.1 ## 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) <h4> All tests should be run against the port production environment(using a testing org). </h4> ### 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. --------- Co-authored-by: tankilevitch <tomtankilevitch@gmail.com>
1 parent fd553e0 commit dbff904

File tree

4 files changed

+75
-83
lines changed

4 files changed

+75
-83
lines changed

integrations/gitlab/CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,14 @@ this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
77

88
<!-- towncrier release notes start -->
99

10+
0.2.28 (2025-01-27)
11+
===================
12+
13+
### Bug Fixes
14+
15+
- Revert changes made in 0.2.1
16+
17+
1018
0.2.27 (2025-01-23)
1119
===================
1220

integrations/gitlab/gitlab_integration/events/event_handler.py

Lines changed: 55 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
event_context,
1616
EventContext,
1717
)
18-
import time
1918

2019
Observer = Callable[[str, dict[str, Any]], Awaitable[Any]]
2120

@@ -29,53 +28,51 @@ def __init__(self) -> None:
2928
async def _start_event_processor(self) -> None:
3029
logger.info(f"Started {self.__class__.__name__} worker")
3130
while True:
32-
event_ctx, event_id, body = await self.webhook_tasks_queue.get()
33-
logger.debug(
34-
f"Retrieved event: {event_id} from Queue, notifying observers",
35-
queue_size=self.webhook_tasks_queue.qsize(),
36-
)
37-
try:
38-
async with event_context(
39-
"gitlab_http_event_async_worker", parent_override=event_ctx
40-
):
41-
await self._notify(event_id, body)
42-
except Exception as e:
43-
logger.error(
44-
f"Error notifying observers for event: {event_id}, error: {e}"
45-
)
46-
finally:
47-
logger.info(
48-
f"Processed event {event_id}",
49-
event_id=event_id,
50-
event_context=event_ctx.id,
31+
event_ctx, event, body = await self.webhook_tasks_queue.get()
32+
with logger.contextualize(
33+
event_context=event_ctx.id,
34+
event_type=event_ctx.event_type,
35+
event_id=event_ctx.id,
36+
event=event,
37+
):
38+
logger.debug(
39+
f"Retrieved event: {event} from Queue, notifying observers",
40+
queue_size=self.webhook_tasks_queue.qsize(),
5141
)
52-
self.webhook_tasks_queue.task_done()
42+
try:
43+
async with event_context(
44+
"gitlab_http_event_async_worker", parent_override=event_ctx
45+
):
46+
await self._notify(event, body)
47+
except Exception as e:
48+
logger.error(
49+
f"Error notifying observers for event: {event}, error: {e}"
50+
)
51+
finally:
52+
logger.info(
53+
f"Processed event {event}",
54+
)
55+
self.webhook_tasks_queue.task_done()
5356

5457
async def start_event_processor(self) -> None:
5558
asyncio.create_task(self._start_event_processor())
5659

5760
@abstractmethod
58-
async def _notify(self, event_id: str, body: dict[str, Any]) -> None:
61+
async def _notify(self, event: str, body: dict[str, Any]) -> None:
5962
pass
6063

61-
async def notify(self, event_id: str, body: dict[str, Any]) -> None:
62-
logger.debug(
63-
f"Received event: {event_id}, putting it in Queue for processing",
64-
event_context=current_event_context.id,
65-
)
64+
async def notify(self, event: str, body: dict[str, Any]) -> None:
65+
logger.debug(f"Received event: {event}, putting it in Queue for processing")
6666
await self.webhook_tasks_queue.put(
6767
(
6868
deepcopy(current_event_context),
69-
event_id,
69+
event,
7070
body,
7171
)
7272
)
7373

7474

7575
class EventHandler(BaseEventHandler):
76-
MAXIMUM_RETRIES = 3
77-
TIMEOUT = 90
78-
7976
def __init__(self) -> None:
8077
super().__init__()
8178
self._observers: dict[str, list[Observer]] = defaultdict(list)
@@ -84,46 +81,24 @@ def on(self, events: list[str], observer: Observer) -> None:
8481
for event in events:
8582
self._observers[event].append(observer)
8683

87-
async def _notify(self, event_id: str, body: dict[str, Any]) -> None:
88-
observers_list = self._observers.get(event_id, [])
84+
async def _notify(self, event: str, body: dict[str, Any]) -> None:
85+
observers_list = self._observers.get(event, [])
86+
8987
if not observers_list:
9088
logger.info(
91-
f"event: {event_id} has no matching handler. the handlers available are for events: {self._observers.keys()}"
89+
f"event: {event} has no matching handler. the handlers available are for events: {self._observers.keys()}"
9290
)
9391
return
9492
for observer in observers_list:
95-
retries_left = self.MAXIMUM_RETRIES
96-
observer_time = time.time()
97-
while retries_left > 0:
98-
try:
99-
if asyncio.iscoroutinefunction(observer):
100-
if inspect.ismethod(observer):
101-
handler = observer.__self__.__class__.__name__
102-
logger.debug(
103-
f"Notifying observer: {handler}, for event: {event_id} at {observer_time}",
104-
event_id=event_id,
105-
handler=handler,
106-
)
107-
await asyncio.wait_for(
108-
observer(event_id, body), self.TIMEOUT
109-
) # Sequentially call each observer
110-
logger.debug(
111-
f"Observer {handler} completed work at {time.time() - observer_time}",
112-
event_id=event_id,
113-
handler=handler,
114-
)
115-
break
116-
except asyncio.TimeoutError:
117-
logger.error(
118-
f"{handler} started work at {observer_time}, did not complete handling event {event_id} within {self.TIMEOUT} seconds, retrying"
119-
)
120-
retries_left -= 1
121-
except Exception as e:
122-
logger.error(
123-
f"Error processing event {event_id} with observer {observer}: {e}",
124-
exc_info=True,
93+
if asyncio.iscoroutinefunction(observer):
94+
if inspect.ismethod(observer):
95+
handler = observer.__self__.__class__.__name__
96+
logger.debug(
97+
f"Notifying observer: {handler}, for event: {event}",
98+
event=event,
99+
handler=handler,
125100
)
126-
break
101+
asyncio.create_task(observer(event, deepcopy(body))) # type: ignore
127102

128103

129104
class SystemEventHandler(BaseEventHandler):
@@ -139,17 +114,20 @@ def on(self, hook_handler: Type[HookHandler]) -> None:
139114
def add_client(self, client: GitlabService) -> None:
140115
self._clients.append(client)
141116

142-
async def _notify(self, event_id: str, body: dict[str, Any]) -> None:
117+
async def _notify(self, event: str, body: dict[str, Any]) -> None:
143118
# best effort to notify using all clients, as we don't know which one of the clients have the permission to
144119
# access the project
145-
for client in self._clients:
146-
for hook_handler_class in self._hook_handlers.get(event_id, []):
147-
try:
148-
hook_handler_instance = hook_handler_class(client)
149-
await hook_handler_instance.on_hook(
150-
event_id, body
151-
) # Sequentially process handlers
152-
except Exception as e:
153-
logger.error(
154-
f"Error processing event {event_id} with handler {hook_handler_class.__name__} for client {client}: {str(e)}"
155-
)
120+
results = await asyncio.gather(
121+
*(
122+
hook_handler(client).on_hook(event, deepcopy(body))
123+
for client in self._clients
124+
for hook_handler in self._hook_handlers.get(event, [])
125+
),
126+
return_exceptions=True,
127+
)
128+
129+
for result in results:
130+
if isinstance(result, Exception):
131+
logger.error(
132+
f"Failed to notify observer for event: {event}, error: {result}"
133+
)

integrations/gitlab/gitlab_integration/gitlab_service.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,9 @@ async def _get_entities_from_git(
228228
project.files.get, file_path, sha
229229
)
230230

231-
entities = yaml.safe_load(file_content.decode())
231+
entities = await anyio.to_thread.run_sync(
232+
yaml.safe_load, file_content.decode()
233+
)
232234
raw_entities = [
233235
Entity(**entity_data)
234236
for entity_data in (
@@ -818,7 +820,7 @@ async def get_entities_diff(
818820

819821
return entities_before, entities_after
820822

821-
def _parse_file_content(
823+
async def _parse_file_content(
822824
self, project: Project, file: ProjectFile
823825
) -> Union[str, dict[str, Any], list[Any]] | None:
824826
"""
@@ -833,13 +835,17 @@ def _parse_file_content(
833835
)
834836
return None
835837
try:
836-
return json.loads(file.decode())
838+
return await anyio.to_thread.run_sync(json.loads, file.decode())
837839
except json.JSONDecodeError:
838840
try:
839841
logger.debug(
840842
f"Trying to process file {file.file_path} in project {project.path_with_namespace} as YAML"
841843
)
842-
documents = list(yaml.load_all(file.decode(), Loader=yaml.SafeLoader))
844+
documents = list(
845+
await anyio.to_thread.run_sync(
846+
yaml.load_all, file.decode(), yaml.SafeLoader
847+
)
848+
)
843849
if not documents:
844850
logger.debug(
845851
f"Failed to parse file {file.file_path} in project {project.path_with_namespace} as YAML,"
@@ -868,7 +874,7 @@ async def get_and_parse_single_file(
868874
f"Fetched file {file_path} in project {project.path_with_namespace}"
869875
)
870876
project_file = typing.cast(ProjectFile, project_file)
871-
parsed_file = self._parse_file_content(project, project_file)
877+
parsed_file = await self._parse_file_content(project, project_file)
872878
project_file_dict = project_file.asdict()
873879

874880
if not parsed_file:

integrations/gitlab/pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[tool.poetry]
22
name = "gitlab"
3-
version = "0.2.27"
3+
version = "0.2.28"
44
description = "Gitlab integration for Port using Port-Ocean Framework"
55
authors = ["Yair Siman-Tov <yair@getport.io>"]
66

0 commit comments

Comments
 (0)