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.

Sign up for GitHub

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

Jump to bottom

6880 import api location docs #6886

Merged
merged 3 commits into from
Nov 5, 2023
Merged

6880 import api location docs #6886

merged 3 commits into from
Nov 5, 2023

Conversation

itaiad200
Copy link
Contributor

closes #6880

@itaiad200 itaiad200 requested review from N-o-Z and kesarwam October 26, 2023 10:50
@itaiad200 itaiad200 added the exclude-changelog PR description should not be included in next release changelog label Oct 26, 2023
N-o-Z
N-o-Z requested changes Oct 26, 2023
View reviewed changes
Copy link
Member

@N-o-Z N-o-Z left a comment

Choose a reason for hiding this comment

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

One inline comment as well as:
#6880 (comment)

api/swagger.yml Outdated
@@ -1250,11 +1250,14 @@ components:
description: Path type, can either be 'common_prefix' or 'object'
path:
type: string
description: A source location to import path or to a single object. Must match the lakeFS installation blockstore type.
description: A source location to a prefix or to a single object. Must match the lakeFS installation blockstore type.
Copy link
Member

Choose a reason for hiding this comment

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

I think we should specify 'common prefix' as we do not support prefix as defined by AWS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@itaiad200 itaiad200 requested a review from N-o-Z November 2, 2023 07:57
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 2, 2023 •
edited
Loading

😭 Deploy PR Preview 111761e failed. Build logs

🤖 By surge-preview

N-o-Z
N-o-Z requested changes Nov 2, 2023
View reviewed changes
Copy link
Member

@N-o-Z N-o-Z left a comment

Choose a reason for hiding this comment

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

Per my previous comment, do we want to modify the import docs as well?

@itaiad200
Copy link
Contributor Author

Per my previous comment, do we want to modify the import docs as well?

@johnnyaug is on it.

@itaiad200 itaiad200 requested a review from N-o-Z November 2, 2023 09:12
N-o-Z
N-o-Z approved these changes Nov 2, 2023
View reviewed changes
Copy link
Member

@N-o-Z N-o-Z left a comment

Choose a reason for hiding this comment

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

LGTM!

@itaiad200 itaiad200 merged commit 700216d into master Nov 5, 2023
31 checks passed
@itaiad200 itaiad200 deleted the 6880-import-api-location-docs branch November 5, 2023 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@N-o-Z N-o-Z N-o-Z approved these changes

@kesarwam kesarwam Awaiting requested review from kesarwam

Assignees
No one assigned
Labels
exclude-changelog PR description should not be included in next release changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Import API of Python Client: "type" field for ImportLocation is not explained
2 participants
@itaiad200 @N-o-Z