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

Fix export test esti job #7635

Merged
merged 1 commit into from
Apr 9, 2024
Merged

Fix export test esti job #7635

merged 1 commit into from
Apr 9, 2024

Conversation

N-o-Z
Copy link
Member

@N-o-Z N-o-Z commented Apr 7, 2024

Closes #7628

The main issue was the dependencies in the container configuration that resulted in another lakeFS instance being brought up and which caused some weird collision.
In addition, added prints and status code checks

@N-o-Z N-o-Z added area/system-tests Improvements or additions to the system-tests exclude-changelog PR description should not be included in next release changelog labels Apr 7, 2024
Copy link

github-actions bot commented Apr 7, 2024

E2E Test Results - DynamoDB Local - Local Block Adapter

10 passed

@N-o-Z N-o-Z force-pushed the fix/enable-rclone-test branch 3 times, most recently from de1ce22 to d770136 Compare April 7, 2024 18:44
@N-o-Z N-o-Z force-pushed the fix/enable-rclone-test branch from d770136 to 2e15b4c Compare April 7, 2024 19:08
@N-o-Z N-o-Z marked this pull request as ready for review April 7, 2024 19:26
@N-o-Z N-o-Z requested a review from guy-har April 7, 2024 19:26
@N-o-Z N-o-Z self-assigned this Apr 7, 2024
Comment on lines -37 to -39
depends_on:
- "lakefs"
- "postgres"
Copy link
Contributor

Choose a reason for hiding this comment

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

Not blocking on this, sharing my thoughts though:
This solves the problem, and because we run it in later steps it works to overcome the dependencies, but we regarding the docker-compose file it's wrong to remove these dependencies.
Did we try using project names?

Copy link
Member Author

Choose a reason for hiding this comment

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

I understand the concern, however:

  1. I feel like we should not invest more time on this since this provides a good enough solution
  2. Lets address this when this actually becomes painful

Comment on lines +6 to +16
run_cmd_and_validate() {
echo $1
echo "Running: $2"
newVariable=$(eval $2)
if [[ $? != "0" ]]
then
echo "FAILED!" && exit 1
fi
echo "Output:"
echo $newVariable
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Like 😎

@N-o-Z N-o-Z merged commit b9a81bc into master Apr 9, 2024
37 checks passed
@N-o-Z N-o-Z deleted the fix/enable-rclone-test branch April 9, 2024 12:00
emulatorchen pushed a commit to emulatorchen/lakeFS that referenced this pull request May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/system-tests Improvements or additions to the system-tests exclude-changelog PR description should not be included in next release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix esti export job
2 participants