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

Specifying a format when reading a datalink should override the format for the target file, not the datalink file #9324

Closed
3 tasks done
radeusgd opened this issue Mar 7, 2024 · 6 comments · Fixed by #9485
Assignees
Labels
-libs Libraries: New libraries to be implemented l-cloud-integration Enso Cloud integration work

Comments

@radeusgd
Copy link
Member

radeusgd commented Mar 7, 2024

Currently, Data.read "./my_csv.datalink" Plain_Text will override the file format and read the datalink definition itself as plain text.

As discussed with @jdunkerley, this is not the desired behaviour. The datalinks should have special handling and instead always resolve as a datalink. If a format is specified, it should override the format that was specified inside of the datalink.

Note: this means that the user cannot easily read datalink definition. To do so, they first need to rename the file to change the extension. Moving/copying the datalink will still copy the target contents, not the datalink description.

  • Change the behaviour for datalinks, overriding the selected format in read.
  • Add a test case for overriding the format.
  • If a datalink is 'format-less' (e.g. Postgres Table), only accept default format (Auto_Detect). Format overrides will result in an error as they are meaningless in such context.
@radeusgd radeusgd added -libs Libraries: New libraries to be implemented l-cloud-integration Enso Cloud integration work labels Mar 7, 2024
@radeusgd radeusgd self-assigned this Mar 7, 2024
@github-project-automation github-project-automation bot moved this to ❓New in Issues Board Mar 7, 2024
@radeusgd radeusgd moved this from ❓New to 📤 Backlog in Issues Board Mar 7, 2024
@radeusgd radeusgd moved this from 📤 Backlog to 🔧 Implementation in Issues Board Mar 13, 2024
@enso-bot
Copy link

enso-bot bot commented Mar 15, 2024

Radosław Waśko reports a new STANDUP for yesterday (2024-03-14):

Progress: Some further debugging, then passed over to Jaroslav who found a fix for the issue that was blocking my PR. Implemented an initial draft of datalink-as-symlinks, but still WIP. It should be finished by 2024-03-20.

Next Day: Next day I will be working on the #8590 task. Serialization is waiting on #9361 so I will try to implement other parts needed for the PR (common IrToTruffle::processName abstraction).

@enso-bot
Copy link

enso-bot bot commented Mar 19, 2024

Radosław Waśko reports a new STANDUP for yesterday (2024-03-18):

Progress: Getting the pending PRs through CI - fixing some tests. Adding datalink checks to primitive file operations. It should be finished by 2024-03-20.

Next Day: Next day I will be working on the same task. Finish the refactor, check that datalinks still work. Add tests for raw operations and datalink copy/move/delete.

@enso-bot
Copy link

enso-bot bot commented Mar 20, 2024

Radosław Waśko reports a new STANDUP for yesterday (2024-03-19):

Progress: Added remaining implementation, updated existing tests - got into a state that existing tests seem to be passing again. It should be finished by 2024-03-20.

Next Day: Next day I will be working on the same task. Add more tests for raw operations (No_Follow) and datalink copy/move/delete (on all backends), datalink from S3 (not to S3).

@enso-bot
Copy link

enso-bot bot commented Mar 21, 2024

Radosław Waśko reports a new 🔴 DELAY for today (2024-03-21):

Summary: There is 2 days delay in implementation of the Specifying a format when reading a datalink should override the format for the target file, not the datalink file (#9324) task.
It will cause 0 days delay for the delivery of this weekly plan.

Delay Cause: Unexpected complexity - only recently we realized we want to handle datalinks in Data.fetch as well - more tests and handling were needed.

@enso-bot
Copy link

enso-bot bot commented Mar 21, 2024

Radosław Waśko reports a new STANDUP for yesterday (2024-03-20):

Progress: Added more tests for copying scenarios, raw config reading etc. Adding tests for Data.fetch and others. It should be finished by 2024-03-22.

Next Day: Next day I will be working on the same task. Some final touches before making the PR ready. Work on Types.

@radeusgd radeusgd moved this from 🔧 Implementation to 👁️ Code review in Issues Board Mar 21, 2024
@enso-bot
Copy link

enso-bot bot commented Mar 22, 2024

Radosław Waśko reports a new STANDUP for yesterday (2024-03-21):

Progress: Adding some more edge case tests. PR in review. Fixed the IllegalStateBug in Types work - I realized I had a misconception about how BindingsMap works - I have now fixed my logic but it needs a bit of a refactor as the code got too ugly for my liking. It should be finished by 2024-03-22.

Next Day: Next day I will be working on the same task. Get the PR merged. Slight refactor for types. Pick a next task.

@mergify mergify bot closed this as completed in #9485 Mar 22, 2024
mergify bot pushed a commit that referenced this issue Mar 22, 2024
@github-project-automation github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-libs Libraries: New libraries to be implemented l-cloud-integration Enso Cloud integration work
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant