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

Follow on from the Enso Cloud API PR. #8352

Closed
14 of 16 tasks
jdunkerley opened this issue Nov 21, 2023 · 7 comments · Fixed by #8497
Closed
14 of 16 tasks

Follow on from the Enso Cloud API PR. #8352

jdunkerley opened this issue Nov 21, 2023 · 7 comments · Fixed by #8497
Assignees
Labels
-libs Libraries: New libraries to be implemented l-cloud-integration Enso Cloud integration work

Comments

@jdunkerley
Copy link
Member

jdunkerley commented Nov 21, 2023

URI_With_Query:

  • Not doing EnsoSecrets in the URI_With_Query.
  • tests for Data.fetch and self.fetch in Http_Spec.
  • tests for Data.post and self.post in Http_Spec.
  • ensure URI has tests for fetch and post methods as well.
  • ensure failures allow a URI_With_Query in the error.
  • refactor the test code to avoid some repitition.
  • Need tests for add_query_arguments URI => URI_With_Query and extending URI_With_Query.
  • Test URI conversion from text.
  • URI_With_Query should support same API as URI so should be run through same set.
  • Character set that needs encoding needs verifying (definitely missing []).
    • Test adding a parameter and sending response.
    • See if simple httpbin would do the test.
    • Test that to_uri works as the original URI_With_Query.

HTTP Util Merge:

  • Test that the basic auth and bearer headers actually work.
  • Should test that changing HTTP version actually works.
  • Headers are an Immutable Dictionary
    • Added workaround in Java for Response
    • Read keys and pass the HTTPHeaders.
    • Ideally felt should be a Map.
    • Response.enso then builds into a Vector
    • check if we can make it work as an Enso Map more directly?
  • Don't think there are any tests for headers being read.
    • add such tests if needed

Missing Tests:

  • JSON into Map had no tests and was broken.

  • (done elsewhere) Add missing Docs on Blank_Selector.

@jdunkerley jdunkerley added the -libs Libraries: New libraries to be implemented label Nov 21, 2023
@jdunkerley jdunkerley added the l-cloud-integration Enso Cloud integration work label Nov 21, 2023
@jdunkerley jdunkerley moved this from ❓New to 📤 Backlog in Issues Board Nov 21, 2023
@radeusgd radeusgd moved this from 📤 Backlog to 🔧 Implementation in Issues Board Dec 4, 2023
@radeusgd
Copy link
Member

radeusgd commented Dec 4, 2023

@jdunkerley I need clarification what did we mean by

tests for Data.fetch and self.fetch in Http_Spec.
?

What was the self?

We have tests for URI.fetch.

Do we want separate tests for Http.new.fetch? These are currently PRIVATE so I assume they are rather internal and it seems enough to have Data.fetch (the public API) tested.

But maybe we meant something else in there, I cannot remember exactly - hence asking.

@radeusgd
Copy link
Member

radeusgd commented Dec 4, 2023

Re: tests for converting text to URI; should we also be able to convert a text to a URI_With_Query? I guess it may make sense to be able to do so, although I'm not sure if it's useful in practice as I don't expect the user constructing URI_With_Query directly - rather getting it as results from the add_query_argument function.

@enso-bot
Copy link

enso-bot bot commented Dec 5, 2023

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

Progress: Refactoring existing tests, adding new tests for HTTP and URI. It should be finished by 2023-12-08.

Next Day: Next day I will be working on the same task. continue

@enso-bot
Copy link

enso-bot bot commented Dec 6, 2023

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

Progress: Adding URI tests, figuring out edge cases worth testing. It should be finished by 2023-12-08.

Next Day: Next day I will be working on the same task. Complete the URI spec, ensure it is all passing. Investigate URI equality issue.

@enso-bot
Copy link

enso-bot bot commented Dec 7, 2023

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

Progress: Implemented URI encoding using Apache helpers, tested all kinds of edge cases now working correctly. URI equality issue was just oversight. Started adding HTTP tests for received headers, HTTP version. Investigated how to test HTTP_Error but no repro scenario found... It should be finished by 2023-12-08.

Next Day: Next day I will be working on the same task. Improve HTTP error code reporting. Add relevant tests. Look into tests for auth header?

@radeusgd
Copy link
Member

radeusgd commented Dec 7, 2023

I amended slightly how headers are handled in the response, but the Enso Response.headers function still returns a Vector Header - that is because the headers may be duplicated and handling of duplicates depends on user's needs. I added an example to headers function showing how to construct a Map from it.

@radeusgd radeusgd moved this from 🔧 Implementation to 👁️ Code review in Issues Board Dec 7, 2023
@enso-bot
Copy link

enso-bot bot commented Dec 8, 2023

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

Progress: Improved HTTP error reporting, added tests for authorization modes, some other edge cases like server responding with no data etc. PR is ready for review. It should be finished by 2023-12-08.

Next Day: Next day I will be working on the #8354 task. Start next ticket

@mergify mergify bot closed this as completed in #8497 Dec 15, 2023
mergify bot pushed a commit that referenced this issue Dec 15, 2023
- Closes #8352
- ~~Proposed fix for #8493~~
- The temporary fix is deemed not viable. I will try to figure out a workaround and leave fixing #8493 to the engine team.
@github-project-automation github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Dec 15, 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 l-cloud-integration Enso Cloud integration work
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants