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

added a flag for if the file is a local path or not #369

Closed
wants to merge 5 commits into from

Conversation

nh916
Copy link
Contributor

@nh916 nh916 commented Sep 26, 2023

Description

added a flag for if the file is a local path or not

Changes

  • changed property setter to a method setter
    • setter takes new_source and a boolean flag of if the file is local or not
  • changed constructor to require boolean flag of is_file_source_local_path
  • wrote set_file_source() docstring sna documentation
  • updated source property docstrings documentation to reflect the new changes
  • changed _upload_file_and_get_object_name to work with boolean flag
    • removed redundant check on it
  • changed is statement check on ensure_uploaded() to work with boolean flag
  • fixed supporting_nodes.py file fixture to work with new boolean flag is_file_source_local_path
  • fixed test_file.py to pass with new boolean flag is_file_source_local_path

Known Issues

Notes

Checklist

  • My name is on the list of contributors (CONTRIBUTORS.md) in the pull request source branch.
  • I have updated the documentation to reflect my changes.

* changed property setter to a method setter
  * setter takes `new_source` and a boolean flag of if the file is local or not
* changed constructor to require boolean flag of `is_file_source_local_path`
* wrote `set_file_source()` docstring sna documentation
* updated source property docstrings documentation to reflect the new changes
* changed `_upload_file_and_get_object_name` to work with boolean flag
  * removed redundant check on it
* changed is statement check on `ensure_uploaded()` to work with boolean flag
@trunk-io
Copy link

trunk-io bot commented Sep 26, 2023

Merging to develop in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

@nh916
Copy link
Contributor Author

nh916 commented Sep 26, 2023

@InnocentBug can't add boolean flag for is_file_source_local_path because then serialization/deserialization will break because it will not have that variable within the JSON when deserializing

we could only have it as a setter though?
I think that could work, but file creation would be a 2 step process, but its pretty simple set I think, and we would add a property getter and setter for is_file_source_local_path

@InnocentBug InnocentBug deleted the add-local-file-flag branch March 27, 2024 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants