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

feat: allow fetching certain entities at a specific revision id #6

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

m90
Copy link
Contributor

@m90 m90 commented Jul 31, 2024

This allows users to pass an optional revid for each entity, locking the imported data to a fixed point in time.

In our setup this is required to fetch data from wikidata like it was before the introduction of the mul langiuage code.

@@ -1,6 +1,6 @@
FROM node:20-alpine

RUN apk add --no-cache curl=8.8.0-r0 python3=3.12.3-r1 && \
RUN apk add --no-cache curl=8.9.0-r0 python3=3.12.3-r1 && \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is unrelated to the feature, but was required to get the image building again.

# "Q42" which fetches the latest revision
# "Q42@123" which fetches revision 123
for entity in $@; do
case $entity in
Copy link

Choose a reason for hiding this comment

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

looks good and functional but I had to remind myself exactly how this case and pattern matching stuff works in bash

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another option would be comparing the output of cut -d "@" -f 1 and cut -d "@" -f 2. If they are the same, there is no @ present. Would you find this easier to digest as a reader?

Copy link

@tarrow tarrow 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 to me; let's see this work.

My general "worries" is that we now have semi-nontrivial stuff in this image in bash, python and javascript but let's not worry too hard about this now.

@m90
Copy link
Contributor Author

m90 commented Jul 31, 2024

My general "worries" is that we now have semi-nontrivial stuff in this image in bash, python and javascript but let's not worry too hard about this now.

I've been thinking the same. In an ideal world, we'd use wb-cli as a library and have all logic live in a big node script. Maybe I can do that still when we know the feature is working.

@m90 m90 merged commit 8097ea0 into main Jul 31, 2024
4 checks passed
@m90 m90 deleted the fr/entity-with-revid branch July 31, 2024 10:39
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