-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
metadata-service [orchestrator]: improve stale metadata detection #42962
metadata-service [orchestrator]: improve stale metadata detection #42962
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @alafanechere and the rest of your teammates on |
6537e9b
to
cbeab52
Compare
44186a8
to
6cf4490
Compare
6cf4490
to
259a837
Compare
259a837
to
5f21fc9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't tested, but the code looks solid
@@ -189,7 +189,7 @@ | |||
), | |||
ScheduleDefinition(job=generate_connector_test_summary_reports, cron_schedule="@hourly"), | |||
ScheduleDefinition( | |||
cron_schedule="0 8 * * *", # Daily at 8am US/Pacific | |||
cron_schedule="0 * * * *", # Every hour |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is nice!
@@ -1,6 +1,6 @@ | |||
[tool.poetry] | |||
name = "orchestrator" | |||
version = "0.1.4" | |||
version = "0.1.5" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: That's prob a 0.2 at least in my book.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I consider it's a fix of something which was working before, this is why I made a patch bump.
@@ -130,7 +130,7 @@ def metadata_definitions(context: OpExecutionContext) -> List[LatestMetadataEntr | |||
metadata_entry = LatestMetadataEntry( | |||
metadata_definition=metadata_def, | |||
icon_url=icon_url, | |||
last_modified=blob.last_modified, | |||
last_modified=blob.updated.isoformat(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious, non-blocking: is this for formatting purposes mostly here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the LastMetadataEntry.last_modified
attribute takes a string - I did not bother changing it to a datetime.
latest_versions_on_gcs = { | ||
metadata_entry.metadata_definition.data.dockerRepository: metadata_entry.metadata_definition.data.dockerImageTag | ||
for metadata_entry in metadata_definitions | ||
if metadata_entry.metadata_definition.data.supportLevel != "archived" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice touch with archived connectors.
5f21fc9
to
71e2ef6
Compare
What
Closes to https://github.com/airbytehq/airbyte-internal-issues/issues/8985
Problem
Since the orchestrator now adds generated field to metadata file the MD5 hash / etag comparison of
master
vsGCS
does not work anymore.Solution
master
branchdockerImageTag
on GCS is different from what we have onmaster
for more than 2 hours.Other changes
The daily frequency (and no message on success) of the stale metadata detection jobs give the false impression that it's not working.
I'd like to increase its frequency (hourly) and make it send a message to a slack even when no stale metadata was detected. This will make sure we have reassuring ping that this alerting is working.