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

Store the script.ID in the router entity? #1085

Closed
spacewander opened this issue Dec 21, 2020 · 18 comments
Closed

Store the script.ID in the router entity? #1085

spacewander opened this issue Dec 21, 2020 · 18 comments
Assignees
Labels
backend enhancement New feature or request wait for update 1. Solution 2. Due date 3. Assignees if needed

Comments

@spacewander
Copy link
Member

It seems currently there is not reference between /apisix/route and the /apisix/script?

@tokers
Copy link
Contributor

tokers commented Dec 21, 2020

That's really, we may add the reference relations just like route and upstream.

@nic-chen
Copy link
Member

sure, we should store it.

@juzhiyuan juzhiyuan added backend enhancement New feature or request labels Dec 21, 2020
@membphis
Copy link
Member

yes, we need this feature ^_^

APISIX needs to be updated for this feature, all right? @spacewander

@spacewander
Copy link
Member Author

Yes.

@juzhiyuan
Copy link
Member

any update?

@juzhiyuan juzhiyuan added stale wait for update 1. Solution 2. Due date 3. Assignees if needed and removed stale labels Jan 4, 2021
@spacewander
Copy link
Member Author

APISIX already updated for this feature

@juzhiyuan
Copy link
Member

ok, waiting manager-api's update.

@membphis
Copy link
Member

ping @nic-chen

@juzhiyuan
Copy link
Member

done??

@imjoey
Copy link
Member

imjoey commented Jan 17, 2021

done??

@juzhiyuan I will work on this if no one has taken.

@juzhiyuan
Copy link
Member

All right, thanks!

@imjoey imjoey self-assigned this Jan 27, 2021
@imjoey
Copy link
Member

imjoey commented Jan 27, 2021

APISIX already updated for this feature

@spacewander AFAIK, APISIX seems not going to deal with loading script by script_id from etcd, which acts not the same as upstream_id. So just to confirm with you that in manager-api, we must fulfill the script property if the input only contains the script_id. Please let me know if I'm wrong. Thanks.

@spacewander
Copy link
Member Author

The script_id is planned to used for debug and optimization(cache). Currently we can assume the script_id is the route's id.

@imjoey
Copy link
Member

imjoey commented Jan 27, 2021

The script_id is planned to used for debug and optimization(cache). Currently we can assume the script_id is the route's id.

@spacewander thanks for getting back. So could we have the following assumptions for now:

  • Currently users could not manipulate the script via manager-api, which means there are no explicite CURDL APIs for script object.
  • If the script_id in request does not equal to the id of the affiliated route, we can directly send back the Bad Request response.

Thanks.

@spacewander
Copy link
Member Author

I think just filling the script_id field with the route id when the script is saved is enough.

@imjoey
Copy link
Member

imjoey commented Jan 27, 2021

I think just filling the script_id field with the route id when the script is saved is enough.

@spacewander Wow, that's great. Many thanks for clear directions.

@imjoey
Copy link
Member

imjoey commented Jan 28, 2021

@spacewander @tokers @juzhiyuan

For now, APISIX and manager-api have both supported script_id in Route entity. Cause The script_id is planned to used for debug and optimization(cache). Currently we can assume the script_id is the route's id., IMHO there is no need to do anything else in manager FE, so could we close this issue?

@juzhiyuan
Copy link
Member

agree with u.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend enhancement New feature or request wait for update 1. Solution 2. Due date 3. Assignees if needed
Projects
None yet
Development

No branches or pull requests

6 participants