-
Notifications
You must be signed in to change notification settings - Fork 19
feat(PoC): adjust file-based and file uploader component to latest protocol changes. #457
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
Changes from all commits
ac278b9
7a02ec8
962ddbe
05bc2cb
3565455
188f9a5
68480b7
6ebfc05
b7bfef5
6445211
2cbd065
69b7835
682318e
8701c27
9fb3b8c
61ccf7c
aa1f394
abeba93
e68873a
2be82e9
8080867
6db0406
199d99a
8920154
cb5884e
fe77b13
b4ce3fa
e746feb
8d567ac
703c268
18a31d2
682598b
f8179ec
9369f26
c758f3b
9dc319e
a4156fb
8693830
8623551
a50905b
4658790
0ac0d5c
e9474c7
f144f98
21b6413
47abb52
3fc844c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
# | ||
# Copyright (c) 2025 Airbyte, Inc., all rights reserved. | ||
# | ||
|
||
from datetime import datetime | ||
from typing import Optional | ||
|
||
from pydantic.v1 import BaseModel | ||
|
||
|
||
class FileRecordData(BaseModel): | ||
""" | ||
A record in a file-based stream. | ||
""" | ||
|
||
folder: str | ||
filename: str | ||
bytes: int | ||
source_uri: str | ||
id: Optional[str] = None | ||
created_at: Optional[str] = None | ||
updated_at: Optional[str] = None | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we also have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, makes sense, I will add it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is now there |
||
mime_type: Optional[str] = None |
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.
Maybe future us problem: Can/Should we allow for additional properties so that sources can add whatever they want? If we do, I think it'll have impact on airbyte_cdk/sources/file_based/schema_helpers.py which seems a bit more challenging as it can't be modified based on the source as it is right now
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.
For now, this is not a concern per the discussion in our last meetings with the team.
Users are getting this metadata "for free" (nobody requested it), but if we need to allow it in the future, we could provide an interface in stream reader similar to what we do to model permissions schema.
I agree this would mean maintenance, but we don't have a current use case, so my belief is that we are ok as it is.
cc @girarda @tryangul
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.
If allowing additional fields is difficult, I'd suggest adding an "additional_properties" field that sources can populate however they want
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.
Okay, that would make sense. I will write a follow-up issue in the epic for this, as I don't see it as a must for this iteration.
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.
Ok, issue created. I also added
created_at
to the schema, I feel this should be enough for this iteration.