-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
scandir and md5 for adlsgen2setup.py #2113
base: main
Are you sure you want to change the base?
Conversation
@microsoft-github-policy-service agree |
scripts/adlsgen2setup.py
Outdated
for directory, directory_client in directories.items(): | ||
directory_path = os.path.join(self.data_directory, directory) | ||
|
||
# Überprüfen, ob 'scandir' existiert und auf False gesetzt ist |
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.
German comment?
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.
Ja! will translate that ;)
scripts/adlsgen2setup.py
Outdated
for file in files: | ||
#Checks to see if the root is '.' and changes it to the correct current | ||
#working directory by calling os.getcwd(). Otherwise root_path will just be the root variable value. | ||
if root == '.': |
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.
Do we only need to set this once? or does it change for every file
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.
yes, good point
scripts/adlsgen2setup.py
Outdated
logger.info(f"Skipping directory {directory} as 'scandir' is set to False") | ||
continue | ||
|
||
groups = self.data_access_control_format["directories"][directory].get("groups", []) |
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.
groups is not used?
scripts/adlsgen2setup.py
Outdated
|
||
async def check_md5(self, path: str, md5_hash: str) -> bool: | ||
# if filename ends in .md5 skip | ||
if path.endswith(".md5"): |
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.
We actually want to move the storage of the MD5 into the Blob storage metadata for our main prepdocs strategy, as the local md5 is problematic when you switch environments. Would that be possible with ADSL2? I assume it also has the ability to store metadata, given its a type of Blob storage?
What do you think of that approach, remote MD5?
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.
I’ve already implemented that and will include it with some additional metadata in the next commit. If i read the
file directly from the source (local storage or blob/data lake storage) and create a new checksum on the fly, keep it in heap . Then, i can compare it with the persistent checksum on the target storage (datalake)and update the blob only if diff and as well the checksum in the meta.
I’ve also started on preodocs.sh to make use of the persist MD5 on the backend once it’s "injected" from the data lake using the datalake strategy which seems to be unstable for exceptions . (exits the whole loop what is even more painful if you don't have a resume) Md5 will not dupe index and tokenizing work - ok, but it still will need to iterate again over all the offset until where the ingestion died abnormally. For external systems i use queries with timestamps to find a proper point to resume but i have no idea if and how current data lake file walker creates any order or if is completely random.
Maybe there is a way to walk/iterate along a query based on a metadata field (i have added) like last change date in metadata? This way i could persist the last ingestion date seen in another blob and use it as resume
I’m unsure where best to store this:
Two options i had in mind
a) In the index, where it would be associated with each chunk, meaning we’d need to locate chunks where there are more than zero results for the fingerprint, updating multiple rows accordingly.
b) In /content (blob storage) for inline rendering of the source document as a citation in the browser. This has the benefit of only requiring one update per change, though we’d still need to locate and remove all matching chunks in the index by primary key for consistency.
Option (a) seems more logical, doesn’t it?
Btw i remember i had trouble with md5 dupes for bigger data amount , so maybe its better to use sha digests?
And even more important: how would above efforts fit into integrated-vectorization?
Will that work with whatever fingerprint being takeover into the index. I assume it requires a new skill, doesn't it?
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.
Blob storage has embedded md5 already as it seems -
from azure.storage.blob import BlobServiceClient
# Your connection string
connection_string = "your_connection_string"
# Your blob container name and blob name
container_name = "your_container_name"
blob_name = "your_blob_name"
# Initialize a BlobServiceClient
blob_service_client = BlobServiceClient.from_connection_string(connection_string)
# Get a reference to the blob
blob_client = blob_service_client.get_blob_client(container=container_name, blob=blob_name)
# Get the properties of the blob
blob_properties = blob_client.get_blob_properties()
# Retrieve the MD5 hash value from the properties
md5_hash = blob_properties.content_settings.content_md5
print(f"MD5 Hash of the blob: {md5_hash}")
If that md5 will have same value as a local computed md5 from file is the question
Also as long as we use as blob name some filename (instead of md5) we still need to rely on that and can't handle rename scenarios. Also we can't easily with O(1) find out if we have any blob up there for the md5 of the file in our hands. If we would use md5 as blob name it would make things in code much slimmer, but debugging and using the azure portal ui file browser for debugging a bit more cumberstone. Also md5 would really need to be unique. her i would only trust sha256 in final instance
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.
Blob service does have its own md5, but it only computes it for small files, so we would need to compute our own hash. Using sha256 also seems fine if we can store that in the blob metadata.
Check Broken URLsWe have automatically detected the following broken URLs in your files. Review and fix the paths to resolve this issue. Check the file paths and associated broken URLs inside them. For more details, check our Contributing Guide.
|
Check Broken URLsWe have automatically detected the following broken URLs in your files. Review and fix the paths to resolve this issue. Check the file paths and associated broken URLs inside them. For more details, check our Contributing Guide.
|
1 similar comment
Check Broken URLsWe have automatically detected the following broken URLs in your files. Review and fix the paths to resolve this issue. Check the file paths and associated broken URLs inside them. For more details, check our Contributing Guide.
|
Check Broken URLsWe have automatically detected the following broken URLs in your files. Review and fix the paths to resolve this issue. Check the file paths and associated broken URLs inside them. For more details, check our Contributing Guide.
|
Check Broken PathsWe have automatically detected the following broken relative paths in your files. Check the file paths and associated broken paths inside them. For more details, check our Contributing Guide.
|
Check Broken URLsWe have automatically detected the following broken URLs in your files. Review and fix the paths to resolve this issue. Check the file paths and associated broken URLs inside them. For more details, check our Contributing Guide.
|
Check Broken URLsWe have automatically detected the following broken URLs in your files. Review and fix the paths to resolve this issue. Check the file paths and associated broken URLs inside them. For more details, check our Contributing Guide.
|
@pamelafox @mattgotteiner still wishes for this PR? |
see #2065