Add `Unknown` type to `TaskInfoType` #542

postmeback · 2024-06-27T04:23:14Z

Pull Request

Related issue

Fixes #385

What does this PR do?

Currently, it adds the "Unknown" type to the enum. It requires more tasks to do

PR checklist

Please check if your PR fulfills the following requirements:

Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
Have you read the contributing guidelines?
Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

curquiza

As discussed, can you add tests? 😊

postmeback · 2024-06-27T12:41:41Z

Yeah, I will. This PR will get updated soon.

postmeback · 2024-06-28T09:51:12Z

Yeah, I will. This PR will get updated soon.

I have updated this PR, to the best of my capabilities. Please review and provide feedback.

postmeback · 2024-07-02T18:05:15Z

Seems like the build failed, Can you guide me on where to look?
@curquiza

curquiza · 2024-07-03T10:20:50Z

@postmeback
The tests on main are green, so it's something you added that broke the CI.
Please, run the tests locally, you will have a better view of what is happening to fix them.

postmeback · 2024-07-04T18:38:16Z

Well, I tried to run the tests, and below is the response.I have commented my tests.

It seems I am restricted. Any idea on how to get past it ?

curquiza · 2024-07-06T15:55:40Z

@postmeback there is not need for access to run the tests locally.
Did you follow the steps in the CONTRIBUTING.md?

postmeback · 2024-07-07T11:52:54Z

I followed the steps and came up with below conclusion

CreateUnknownTaskAsync & GetTaskWithUnknownTypeAsync - this two methods have an URI, which I created and not added with API, that's the reason it is failing.

https://github.com/postmeback/meilisearch-dotnet/blob/c2934b51bae7b0f243009af109f26499cb7585a0/src/Meilisearch/MeilisearchClient.cs#L430

https://github.com/postmeback/meilisearch-dotnet/blob/c2934b51bae7b0f243009af109f26499cb7585a0/src/Meilisearch/MeilisearchClient.cs#L438C37-L438C64

Requesting you to have a look.

curquiza · 2024-07-08T12:49:21Z

@postmeback CreateUnknownTaskAsync call a route that does not exist in Meilisearch. It cannot work.

Maybe @ahmednfwela can guide you on this, or @brunoocasali who opened the issue.

ahmednfwela

to address the issue, we just need to configure the json serializer used by meilisearch to fallback to the unknown enum value

ahmednfwela · 2024-07-09T08:03:54Z

src/Meilisearch/MeilisearchClient.cs

+
+
+ public async Task<TaskInfo> CreateUnknownTaskAsync(UnknownTaskQuery query, CancellationToken cancellationToken = default)
+ {
+ var response = await _http.PostAsJsonAsync("create-unknown-task", query, Constants.JsonSerializerOptionsRemoveNulls, cancellationToken: cancellationToken)
+ .ConfigureAwait(false);
+
+ return await response.Content.ReadFromJsonAsync<TaskInfo>(cancellationToken: cancellationToken).ConfigureAwait(false);
+ }
+
+ public async Task<TaskInfo> GetTaskWithUnknownTypeAsync(GetTaskQuery query, CancellationToken cancellationToken = default)
+ {
+ var response = await _http.PostAsJsonAsync("get-task-with-unknown-type", query, Constants.JsonSerializerOptionsRemoveNulls, cancellationToken: cancellationToken)
+ .ConfigureAwait(false);
+
+ return await response.Content.ReadFromJsonAsync<TaskInfo>(cancellationToken: cancellationToken).ConfigureAwait(false);
+ }
+


all of these are unnecessary

ahmednfwela · 2024-07-09T08:04:33Z

src/Meilisearch/QueryParameters/GetTaskQuery.cs

@@ -0,0 +1,12 @@
+using System;


this new file is unnecessary

ahmednfwela · 2024-07-09T08:04:50Z

src/Meilisearch/QueryParameters/UnknownTaskQuery.cs

@@ -0,0 +1,13 @@
+using System;


this new file is unnecessary

postmeback · 2024-07-11T05:49:16Z

I have removed the unnecessary files and methods from this PR.

Also, if you can point to the json serializer file/function, which needs to be updated, that would greatly assist. @ahmednfwela

ahmednfwela · 2024-07-11T06:17:25Z

@postmeback we use the default one https://learn.microsoft.com/en-us/dotnet/api/system.text.json.serialization.jsonstringenumconverter?view=net-8.0

if this converter doesn't have a fallback option, we can create a custom one that inherits it

ahmednfwela · 2024-07-11T06:35:47Z

check this SO question for similar solutions

postmeback · 2024-07-16T18:06:42Z

Sure, thanks for the suggestion. I will update you soon.

postmeback · 2024-07-30T17:46:30Z

With the latest changes and all tests passing

postmeback · 2024-08-01T18:14:34Z

I ran the formatter as well.

ahmednfwela

can you please replace the json converter annotation on top of the enum with the new converter

postmeback · 2024-08-03T11:02:51Z

Done.

ahmednfwela · 2024-08-03T16:32:27Z

bors merge

542: Add `Unknown` type to `TaskInfoType` r=ahmednfwela a=postmeback # Pull Request ## Related issue Fixes #385 ## What does this PR do? - Currently, it adds the "Unknown" type to the enum. It requires more tasks to do ## PR checklist Please check if your PR fulfills the following requirements: - [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)? - [x] Have you read the contributing guidelines? - [x] Have you made sure that the title is accurate and descriptive of the changes? Thank you so much for contributing to Meilisearch! Co-authored-by: postmeback <[email protected]> Co-authored-by: Clémentine <[email protected]> Co-authored-by: Ahmed Fwela <[email protected]> Co-authored-by: Arka Poddar <[email protected]>

meili-bors · 2024-08-03T16:34:57Z

This PR was included in a batch that successfully built, but then failed to merge into main. It will not be retried.

Additional information:

{"message":"1 review requesting changes and 1 approving review by reviewers with write access.","documentation_url":"https://docs.github.com/articles/about-protected-branches","status":"422"}

curquiza

missing my approval

bors merge

meili-bors · 2024-08-05T07:59:23Z

Build succeeded:

Unknown Type Added

958968e

curquiza requested changes Jun 27, 2024

View reviewed changes

curquiza added the enhancement New feature or request label Jun 27, 2024

curquiza changed the title ~~Unknown Type Added~~ Add Unknown type to TaskInfoType Jun 27, 2024

curquiza changed the title ~~Add Unknown type to TaskInfoType~~ Add Unknown type to TaskInfoType Jun 27, 2024

Added tests and additional classes

1154c04

Moved file positions

c2934b5

Merge branch 'main' into main

f966d1f

ahmednfwela requested changes Jul 9, 2024

View reviewed changes

postmeback added 2 commits July 11, 2024 11:08

Removed unnecessary files and methods

6ce326f

Merge branch 'main' of https://github.com/postmeback/meilisearch-dotnet

ffb8a86

ahmednfwela and others added 3 commits July 11, 2024 14:07

Merge branch 'main' into main

9f0c618

Removed unnecessary tests

7190f79

Merge branch 'main' of https://github.com/postmeback/meilisearch-dotnet

71e432d

postmeback added 2 commits July 30, 2024 23:17

Changes Done

460563d

Ran formatter

bdf0b29

Merge branch 'main' into main

3b84c94

curquiza requested a review from ahmednfwela August 2, 2024 15:23

ahmednfwela requested changes Aug 2, 2024

View reviewed changes

postmeback added 2 commits August 3, 2024 16:32

Suggested Changes Done

41c23fa

Merge branch 'main' of https://github.com/postmeback/meilisearch-dotnet

88019e8

curquiza requested a review from ahmednfwela August 3, 2024 15:23

ahmednfwela approved these changes Aug 3, 2024

View reviewed changes

ahmednfwela requested a review from curquiza August 3, 2024 17:45

curquiza approved these changes Aug 5, 2024

View reviewed changes

meili-bors bot merged commit 70a7026 into meilisearch:main Aug 5, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `Unknown` type to `TaskInfoType` #542

Add `Unknown` type to `TaskInfoType` #542

postmeback commented Jun 27, 2024

curquiza left a comment

postmeback commented Jun 27, 2024

postmeback commented Jun 28, 2024

postmeback commented Jul 2, 2024

curquiza commented Jul 3, 2024

postmeback commented Jul 4, 2024

curquiza commented Jul 6, 2024

postmeback commented Jul 7, 2024

curquiza commented Jul 8, 2024

ahmednfwela left a comment

ahmednfwela Jul 9, 2024

ahmednfwela Jul 9, 2024

ahmednfwela Jul 9, 2024

postmeback commented Jul 11, 2024

ahmednfwela commented Jul 11, 2024

ahmednfwela commented Jul 11, 2024

postmeback commented Jul 16, 2024

postmeback commented Jul 30, 2024

postmeback commented Aug 1, 2024

ahmednfwela left a comment

postmeback commented Aug 3, 2024

ahmednfwela commented Aug 3, 2024

meili-bors bot commented Aug 3, 2024

curquiza left a comment

meili-bors bot commented Aug 5, 2024

Add Unknown type to TaskInfoType #542

Add Unknown type to TaskInfoType #542

Conversation

postmeback commented Jun 27, 2024

Pull Request

Related issue

What does this PR do?

PR checklist

curquiza left a comment

Choose a reason for hiding this comment

postmeback commented Jun 27, 2024

postmeback commented Jun 28, 2024

postmeback commented Jul 2, 2024

curquiza commented Jul 3, 2024

postmeback commented Jul 4, 2024

curquiza commented Jul 6, 2024

postmeback commented Jul 7, 2024

curquiza commented Jul 8, 2024

ahmednfwela left a comment

Choose a reason for hiding this comment

ahmednfwela Jul 9, 2024

Choose a reason for hiding this comment

ahmednfwela Jul 9, 2024

Choose a reason for hiding this comment

ahmednfwela Jul 9, 2024

Choose a reason for hiding this comment

postmeback commented Jul 11, 2024

ahmednfwela commented Jul 11, 2024

ahmednfwela commented Jul 11, 2024

postmeback commented Jul 16, 2024

postmeback commented Jul 30, 2024

postmeback commented Aug 1, 2024

ahmednfwela left a comment

Choose a reason for hiding this comment

postmeback commented Aug 3, 2024

ahmednfwela commented Aug 3, 2024

meili-bors bot commented Aug 3, 2024

curquiza left a comment

Choose a reason for hiding this comment

meili-bors bot commented Aug 5, 2024

Add `Unknown` type to `TaskInfoType` #542

Add `Unknown` type to `TaskInfoType` #542