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

Bug fix in the read/write/delete samples with LocalStack #2067

Merged
merged 7 commits into from
Apr 9, 2023

Conversation

aradhalevy
Copy link
Contributor

@aradhalevy aradhalevy commented Apr 8, 2023

Added control over the version of LocalStack that is installed (not just taking the latest) as part of the samples flow.

This way, when LocalStack updates it won't break like happened #2052.

closes #2052

Arad Halevy added 6 commits April 7, 2023 16:22
…k version

Signed-off-by: Arad Halevy <arad.halevy@ibm.com>
…k version across fybrik

Signed-off-by: Arad Halevy <arad.halevy@ibm.com>
…tall

Signed-off-by: Arad Halevy <arad.halevy@ibm.com>
Signed-off-by: Arad Halevy <arad.halevy@ibm.com>
Signed-off-by: Arad Halevy <arad.halevy@ibm.com>
@@ -25,6 +27,7 @@ if [[ "$FYBRIK_VERSION" == "" ]]; then
# Get Fybrik lateset realease from github
FYBRIK_VERSION=$(git -c 'versionsort.suffix=-' ls-remote --tags --sort='v:refname' https://github.com/fybrik/fybrik.git | tail --lines=1 | cut --delimiter='/' --fields=3)
fi
FYBRIK_BRANCH_OMD="$FYBRIK_VERSION"
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do you need a separate FYBRIK_BRANCH_OMD env variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is as it is installed in the QuickStart, it is used for the OpenMetaData script, but it needs to be FYBRIK_BRANCH and exported for it to work for that.
I changed it accordingly.

Copy link
Collaborator

@roytman roytman left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, several tiny comments.

site/docs/samples/notebook-read.md Outdated Show resolved Hide resolved
samples/OneClickDemo/OneClickDemo-OMD.sh Outdated Show resolved Hide resolved
Signed-off-by: Arad Halevy <arad.halevy@ibm.com>
@roytman roytman merged commit 60f5e0c into fybrik:master Apr 9, 2023
@BrodaUa
Copy link

BrodaUa commented Jan 18, 2024

Hello @aradhalevy @roytman

as of version v1.3.2, this fix is missing in OneClickDemo, see:
https://raw.githubusercontent.com/fybrik/fybrik/v1.3.2/samples/OneClickDemo/OneClickDemo-OMD.sh

the same issue appears in the step-by-step guides page here:
https://fybrik.io/v1.3/samples/notebook-read/ (localstack setup does not work - complaints about 'theshire' region)

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.

when running the QuickStart readflow, AWS CLI doesn't accept "theshire" as a Region
3 participants