Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
454 migrate resources from old cms to aiod platform #469
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
base: develop
Are you sure you want to change the base?
454 migrate resources from old cms to aiod platform #469
Changes from 14 commits
3201b3a
3ff6149
387ad73
cba3df3
5bb9e4f
db7d758
1c51694
6bc6b64
51a1700
da4c6dc
fd2fd9a
b0b93bb
ef24a07
8ea3275
b3f04d5
723b981
6b2705c
1422345
094669f
3742286
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
It seems like you are fetching -all- events from ai4eu every hour. This is very wasteful and imposes unneeded strain on both servers and databases, and more prone to connection errors. If you do not fix this from the ai4eu side, please consider at least updating the connector. For example, you could make sure the
date_published
exceeds the last added entry. You can use thestate
dictionary that's passed as an argument to this function for this. The dictionary is saved to a file every x items, and automatically loaded and passed through on the next invocation of the connector.E.g.,
if state.get("last_entry_published", "2000-01-01T00:00:00") > event["last_published"]: continue
at the start of the loop body and setstate["last_entry_published"] = event["date_published"]
after the yield statement should work and at least avoid trying to insert many events which already exist in the database.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 think the more "clear" way to resolve this is by adding the ai4eu identifier into the response of the API. This way we could utilize
ResourceConnectorById
class since we know that every new asset (organisation / news / event) will get an identifier that is higher than the currest highest identifier.@AlexJoom do you have any thoughts on this one? Can we add the identifier into the ai4eu API response?
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.
@antonis96 the ai4europe api response has a
platform_resource_identifier
property.For example page https://ai4europe.eu/node/2420
has
platform_resource_identifier
: "node-2420"Is this sufficient?
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.
@AlexJoom Yes, this is totally fine. I will proceed with the necessary changes.
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.
Safely retrieving the value for a key and otherwise returning None is exactly what
dict.get
does, so you can replace this pattern with just a call todict.get
.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.
Platform and Platform Identifier are specifically for connectors and are limited to exactly those platforms we have connectors for. In this case I think the platform should simply be
PlatformName.ai4europe_cms
with the identifier of the asset in the ai4europe_cms catalogue.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.
In these cases you may also need to use the second parameter which specifies the value to return if the key is not present in the dictionary, e.g.:
are equivalent.