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

Sort output of S3_File.list and Enso_File.list #11929

Merged
merged 25 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions distribution/lib/Standard/AWS/0.0.0-dev/src/Internal/S3_Path.enso
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from Standard.Base import all
import Standard.Base.Errors.Illegal_Argument.Illegal_Argument
import Standard.Base.Internal.Path_Helpers
from Standard.Base.Data.Ordering import default_comparator_hash

import project.Errors.S3_Error
import project.S3.S3
Expand Down Expand Up @@ -157,14 +156,3 @@ type Decomposed_S3_Path
if self.parts.is_empty then Nothing else
new_parts = self.parts.drop (..Last 1)
Decomposed_S3_Path.Value new_parts self.go_to_root

## PRIVATE
type S3_Path_Comparator
## PRIVATE
compare x y = Ordering.compare x.bucket y.bucket . and_then (Ordering.compare x.key y.key)

## PRIVATE
hash x = default_comparator_hash x

## PRIVATE
Comparable.from that:S3_Path = Comparable.new that S3_Path_Comparator
15 changes: 2 additions & 13 deletions distribution/lib/Standard/AWS/0.0.0-dev/src/S3/S3_File.enso
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from Standard.Base import all
import Standard.Base.Data.Ordering.Vector_Lexicographic_Order
import Standard.Base.Enso_Cloud.Data_Link.Data_Link
import Standard.Base.Enso_Cloud.Data_Link_Helpers
import Standard.Base.Errors.Common.Syntax_Error
Expand All @@ -12,7 +13,6 @@ import Standard.Base.System.File.Generic.Writable_File.Writable_File
import Standard.Base.System.File_Format_Metadata.File_Format_Metadata
import Standard.Base.System.Input_Stream.Input_Stream
import Standard.Base.System.Output_Stream.Output_Stream
from Standard.Base.Data.Ordering import default_comparator_hash
from Standard.Base.System.File import find_extension_from_name
from Standard.Base.System.File.Generic.File_Write_Strategy import generic_copy

Expand Down Expand Up @@ -188,7 +188,7 @@ type S3_File
S3_File.Value (S3_Path.Value bucket key) self.credentials
files = pair.second . map key->
S3_File.Value (S3_Path.Value bucket key) self.credentials
(sub_folders + files) . sort
(sub_folders + files) . sort on=(f-> [f.s3_path.bucket, f.s3_path.key]) by=Vector_Lexicographic_Order.compare

## ALIAS load bytes, open bytes
ICON data_input
Expand Down Expand Up @@ -617,14 +617,3 @@ translate_file_errors related_file result =
s3_path = S3_Path.Value error.bucket error.key
s3_file = S3_File.Value s3_path related_file.credentials
Error.throw (File_Error.Not_Found s3_file)

## PRIVATE
type S3_File_Comparator
## PRIVATE
compare x y = Ordering.compare x.s3_path y.s3_path

## PRIVATE
hash x = default_comparator_hash x

## PRIVATE
Comparable.from that:S3_File = Comparable.new that S3_File_Comparator
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,3 @@ type Ordering

## PRIVATE
Comparable.from (that:Ordering) = Comparable.new that Ordering_Comparator

## PRIVATE
default_comparator_hash x = Default_Comparator.hash x
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import project.Any.Any
import project.Data.Color.Color
import project.Data.Json.JS_Object
import project.Data.Ordering.Comparable
import project.Data.Ordering.Ordering
import project.Data.Ordering.Vector_Lexicographic_Order
import project.Data.Numbers.Integer
import project.Data.Text.Encoding.Encoding
import project.Data.Text.Text
Expand Down Expand Up @@ -42,7 +41,6 @@ import project.System.File_Format_Metadata.File_Format_Metadata
import project.System.Input_Stream.Input_Stream
import project.System.Output_Stream.Output_Stream
from project.Data.Boolean import Boolean, False, True
from project.Data.Ordering import default_comparator_hash
from project.Data.Text.Extensions import all
from project.Enso_Cloud.Internal.Enso_File_Helpers import all
from project.Enso_Cloud.Public_Utils import get_required_field
Expand Down Expand Up @@ -451,7 +449,7 @@ type Enso_File
file = Enso_File.Value (self.enso_path.resolve asset.name)
Asset_Cache.update file asset
file
results.sort
results.sort on=(f-> f.enso_path.path_segments) by=Vector_Lexicographic_Order.compare
Copy link
Member

Choose a reason for hiding this comment

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

This is OK, although I'd still prefer to just do on=.path for all backends.

The catch is - if we ever make list with recursive=True allow to cross data links, Enso_File.list could still return S3_File instances if there's a data link pointing towards an S3 bucket. So a file-specific approach to sorting would break in such scenario whereas a .path based one will always be universal.

Although I'm not sure how likely the above scenario is, so for now this is also OK.

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.


## GROUP Output
ICON folder_add
Expand Down Expand Up @@ -587,14 +585,3 @@ File_Like.from (that : Enso_File) = File_Like.Value that
## PRIVATE
Writable_File.from (that : Enso_File) = if Data_Link.is_data_link that then Data_Link_Helpers.interpret_data_link_as_writable_file that else
Writable_File.Value that Enso_File_Write_Strategy.instance

## PRIVATE
type Enso_File_Comparator
## PRIVATE
compare x y = Ordering.compare x.enso_path y.enso_path

## PRIVATE
hash x = default_comparator_hash x

## PRIVATE
Comparable.from that:Enso_File = Comparable.new that Enso_File_Comparator
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
private

import project.Data.Ordering.Comparable
import project.Data.Ordering.Ordering
import project.Data.Ordering.Vector_Lexicographic_Order
import project.Data.Text.Text
import project.Data.Vector.Vector
import project.Enso_Cloud.Enso_File.Enso_File
Expand All @@ -13,7 +10,6 @@ import project.Errors.Unimplemented.Unimplemented
import project.Internal.Path_Helpers
import project.Nothing.Nothing
from project.Data.Boolean import Boolean, False, True
from project.Data.Ordering import default_comparator_hash
from project.Data.Text.Extensions import all

## PRIVATE
Expand Down Expand Up @@ -76,14 +72,3 @@ normalize segments =
## We need to call normalize again, because technically a path `enso://a/../~/../../~` is a valid path
that points to the user home and it should be correctly normalized, but requires numerous passes to do so.
@Tail_Call normalize new_segments

## PRIVATE
type Enso_Path_Comparator
## PRIVATE
compare x y = Vector_Lexicographic_Order.compare x.path_segments y.path_segments

## PRIVATE
hash x = default_comparator_hash x

## PRIVATE
Comparable.from that:Enso_Path = Comparable.new that Enso_Path_Comparator
5 changes: 2 additions & 3 deletions test/AWS_Tests/src/S3_Spec.enso
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,8 @@ add_specs suite_builder =
group_builder.specify "list should sort its output" <|
r = root.list
r.should_be_a Vector
r . should_equal (r.sort on=(x-> x.path))
# Check sorting directly.
r . reverse . sort . should_equal r
as_strings = r.map .to_text
as_strings . should_equal as_strings.sort

group_builder.specify "will fail if no credentials are provided and no Default credentials are available" pending=(if AWS_Credential.is_default_credential_available then "Default AWS credentials are defined in the environment and this test has no way of overriding them, so it is impossible to test this scenario in such environment.") <|
root_without_credentials = S3_File.new "s3://"+bucket_name+"/"
Expand Down
5 changes: 2 additions & 3 deletions test/Base_Tests/src/Network/Enso_Cloud/Enso_File_Spec.enso
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,8 @@ add_specs suite_builder setup:Cloud_Tests_Setup = suite_builder.group "Enso Clou
group_builder.specify "list should sort its output" <|
r = Enso_File.home.list
r.should_be_a Vector
r . should_equal (r.sort on=.path)
# Check sorting directly.
r . reverse . sort . should_equal r
as_strings = r.map .to_text
as_strings . should_equal as_strings.sort

group_builder.specify "should allow to create and delete a directory" <|
my_name = "my_test_dir-" + (Random.uuid.take 5)
Expand Down
Loading