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

Merge URI with URI_With_Query #8544

Closed
radeusgd opened this issue Dec 14, 2023 · 3 comments · Fixed by #8591
Closed

Merge URI with URI_With_Query #8544

radeusgd opened this issue Dec 14, 2023 · 3 comments · Fixed by #8591
Assignees
Labels
-libs Libraries: New libraries to be implemented

Comments

@radeusgd
Copy link
Member

After some followup work and discussion, we conjecture that it may be cleaner to merge the utility of URI_With_Query directly into the URI, to have more uniform handling.

The APIs should be merged, we may want to add following additional methods:

  • to_java_uri - a counterpart of the old URI_With_Query.to_uri that converts to pure Java URI, if there are no secrets - it may be useful for user's interaction with other Java libraries. It will still throw an error if there are secrests - only Enso APIs are allowed to work with those.
  • query_arugments : Vector (Pair Text (Text | Enso_Secret)) - it may be worth to be able to query what arguments are currently in the URI, in the parsed form (counterpart to query : Text which returns the arguments in already encoded form).
    • I think we should remove raw_query as it can cause more harm than good - query_arugments will be more generic and serve the purpose of raw_ better.
    • These query_arugments must report all arguments, both ones parsed by URI.parse as well as added with add_query_argument - there should be no distinction between how an argument was added.
  • the underlying URI instance shall likely hold an Apache URIBuilder instead of raw URI, as that allows to parse the query args and allows for easier manipulation of the URI.
  • while we are at it, it may be worth to add a / operator to URI that allows to extend the path with additional fragments, analogously to File./.
@radeusgd radeusgd added the -libs Libraries: New libraries to be implemented label Dec 14, 2023
@radeusgd radeusgd self-assigned this Dec 14, 2023
@radeusgd radeusgd moved this from ❓New to 🔧 Implementation in Issues Board Dec 14, 2023
@enso-bot
Copy link

enso-bot bot commented Dec 18, 2023

Radosław Waśko reports a new STANDUP for the provided date (2023-12-15):

Progress: Getting all pending PRs forward - fixing some tests, a few already got merged. Work on merging URI with URI_With_Query. It should be finished by 2023-12-20.

Next Day: Next day I will be working on the same task. Continue - creating a common way to create a URI from a 'schematic' that may contain secrets inside of query or user_info. Addressing PR comments.

@enso-bot
Copy link

enso-bot bot commented Dec 19, 2023

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

Progress: Removed Apache dependency. Created a common URI API. Working on making tests pass It should be finished by 2023-12-20.

Next Day: Next day I will be working on the same task. Fix remaining test failures

@radeusgd radeusgd linked a pull request Dec 19, 2023 that will close this issue
5 tasks
@radeusgd radeusgd moved this from 🔧 Implementation to 👁️ Code review in Issues Board Dec 19, 2023
@enso-bot
Copy link

enso-bot bot commented Dec 19, 2023

Radosław Waśko reports a new STANDUP for today (2023-12-19):

Progress: Pulling the PRs thru reviews and CI, merged a lot. Prepared the URI merge PR for review. Discussion with Jaroslav on next steps on types. It should be finished by 2023-12-20.

Next Day: Next day I will be working on the #8549 task. Pick up some next tasks.

@mergify mergify bot closed this as completed in #8591 Dec 21, 2023
mergify bot pushed a commit that referenced this issue Dec 21, 2023
- Closes #8544
- Adds `reset_query_arguments` and `/` operators allowing to transform a URI.
- Adding tests for handling of various edge cases.
@github-project-automation github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Dec 21, 2023
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
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant