You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@texera.apache.org by "Xiao-zhen-Liu (via GitHub)" <gi...@apache.org> on 2025/10/15 23:25:48 UTC
[PR] feat: [chat-assistant-service][1] add backend service skeleton [texera]
Xiao-zhen-Liu opened a new pull request, #3912:
URL: https://github.com/apache/texera/pull/3912
WIP.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(chat-assistant-service): add backend service skeleton [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam commented on code in PR #3912:
URL: https://github.com/apache/texera/pull/3912#discussion_r2440672251
##########
chat-assistant-service/requirements.txt:
##########
Review Comment:
I think when we use `poetry` we shouldn't add `requirements.txt` to the project
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(chat-assistant-service): add backend service skeleton [texera]
Posted by "Xiao-zhen-Liu (via GitHub)" <gi...@apache.org>.
Xiao-zhen-Liu commented on PR #3912:
URL: https://github.com/apache/texera/pull/3912#issuecomment-3420528973
> in your code, `tests` is in `app`. can we move it out? I also suggest you add app into a `src` folder`, and make test folder structure mapped to src folder structure:
>
> ```
>
> ├── src/
> │ ├──app/
> ├── test/
> │ └──app/ # contains tests regarding app
> │ └──── test_health_check.py # Unit tests
> ```
I restructured the project according to your suggestions. An additional `pytest.ini` is needed for this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(chat-assistant-service): add backend service skeleton [texera]
Posted by "Xiao-zhen-Liu (via GitHub)" <gi...@apache.org>.
Xiao-zhen-Liu commented on code in PR #3912:
URL: https://github.com/apache/texera/pull/3912#discussion_r2445656679
##########
chat-assistant-service/pyproject.toml:
##########
@@ -0,0 +1,23 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[tool.black]
+line-length = 88
+target-version = ["py312"]
+
+[tool.flake8]
+max-line-length = 88
Review Comment:
Sure. Let's do this in another PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(chat-assistant-service): add backend service skeleton [texera]
Posted by "Xiao-zhen-Liu (via GitHub)" <gi...@apache.org>.
Xiao-zhen-Liu commented on code in PR #3912:
URL: https://github.com/apache/texera/pull/3912#discussion_r2443792422
##########
chat-assistant-service/pyproject.toml:
##########
@@ -0,0 +1,23 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[tool.black]
+line-length = 88
+target-version = ["py312"]
+
+[tool.flake8]
+max-line-length = 88
Review Comment:
@Yicong-Huang do you know a way to set black configurations without using `pyproject.toml`? If not, I need to keep `pyproject.toml`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: [chat-assistant-service][1] add backend service skeleton [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #3912:
URL: https://github.com/apache/texera/pull/3912#discussion_r2438037395
##########
chat-assistant-service/app/tests/__init__.py:
##########
Review Comment:
I think tests don't need to be a module (no where need to import it). maybe delete this empty `__init__.py`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: [chat-assistant-service][1] add backend service skeleton [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #3912:
URL: https://github.com/apache/texera/pull/3912#discussion_r2438036359
##########
chat-assistant-service/.gitignore:
##########
Review Comment:
do you want to merge this to our main repo's .gitignore?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(chat-assistant-service): add backend service skeleton [texera]
Posted by "Xiao-zhen-Liu (via GitHub)" <gi...@apache.org>.
Xiao-zhen-Liu closed pull request #3912: feat(chat-assistant-service): add backend service skeleton
URL: https://github.com/apache/texera/pull/3912
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(chat-assistant-service): add backend service skeleton [texera]
Posted by "Xiao-zhen-Liu (via GitHub)" <gi...@apache.org>.
Xiao-zhen-Liu commented on code in PR #3912:
URL: https://github.com/apache/texera/pull/3912#discussion_r2443794204
##########
chat-assistant-service/.gitignore:
##########
Review Comment:
Actually main repo's .gitignore covers what are needed for this project. I removed the new, redundant .gitignore.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(chat-assistant-service): add backend service skeleton [texera]
Posted by "Xiao-zhen-Liu (via GitHub)" <gi...@apache.org>.
Xiao-zhen-Liu commented on code in PR #3912:
URL: https://github.com/apache/texera/pull/3912#discussion_r2441452012
##########
chat-assistant-service/requirements.txt:
##########
Review Comment:
What is `poetry`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: [chat-assistant-service][1] add backend service skeleton [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #3912:
URL: https://github.com/apache/texera/pull/3912#issuecomment-3413530865
in your code, `tests` is in `app`. can we move it out so that it can be parallel with app, and to be consistent with your PR description? I also suggest you add app into a `src` folder`:
├── src/
│ ├──app/
├── test/
│ └── test_health_check.py # Unit tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(chat-assistant-service): add backend service skeleton [texera]
Posted by "Xiao-zhen-Liu (via GitHub)" <gi...@apache.org>.
Xiao-zhen-Liu commented on code in PR #3912:
URL: https://github.com/apache/texera/pull/3912#discussion_r2438284138
##########
chat-assistant-service/pyproject.toml:
##########
@@ -0,0 +1,23 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[tool.black]
+line-length = 88
+target-version = ["py312"]
+
+[tool.flake8]
+max-line-length = 88
Review Comment:
`black` is the formatter and not a linter. The purpose of this toml here is not to manage dependencies but to configure black to set the max line length so it can automatically format lines (seems the toml is the only way to do it in a configuration file).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: [chat-assistant-service][1] add backend service skeleton [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #3912:
URL: https://github.com/apache/texera/pull/3912#discussion_r2438048129
##########
chat-assistant-service/pyproject.toml:
##########
@@ -0,0 +1,23 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[tool.black]
+line-length = 88
+target-version = ["py312"]
+
+[tool.flake8]
+max-line-length = 88
Review Comment:
we have been using `black` for linter and requirements.txt for declare dependencies. I know toml is another way to manage python services, for example you can declare dependencies and tools (e.g., linter). However, it seems to be redundant (we have two set of managing services). can we select one to use?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(chat-assistant-service): add backend service skeleton [texera]
Posted by "Xiao-zhen-Liu (via GitHub)" <gi...@apache.org>.
Xiao-zhen-Liu commented on code in PR #3912:
URL: https://github.com/apache/texera/pull/3912#discussion_r2443794475
##########
chat-assistant-service/app/tests/__init__.py:
##########
Review Comment:
Deleted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(chat-assistant-service): add backend service skeleton [texera]
Posted by "yunyad (via GitHub)" <gi...@apache.org>.
yunyad commented on code in PR #3912:
URL: https://github.com/apache/texera/pull/3912#discussion_r2447167394
##########
chat-assistant-service/pyproject.toml:
##########
@@ -0,0 +1,30 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Currently we only use this file for setting tool configurations.
+
+[tool.black]
+line-length = 88
+target-version = ["py312"]
+
+[tool.flake8]
+max-line-length = 88
+
+[tool.pytest.ini_options]
+addopts = "-q"
+pythonpath = ["src"]
Review Comment:
Is pythonpath = ["src"] meant for the pytest-pythonpath plugin?
If so, I don’t see that plugin in the dependencies. Should we add it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(chat-assistant-service): add backend service skeleton [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #3912:
URL: https://github.com/apache/texera/pull/3912#discussion_r2438290484
##########
chat-assistant-service/pyproject.toml:
##########
@@ -0,0 +1,23 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[tool.black]
+line-length = 88
+target-version = ["py312"]
+
+[tool.flake8]
+max-line-length = 88
Review Comment:
see this file https://github.com/apache/texera/blob/main/amber/src/main/python/.flake8
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(chat-assistant-service): add backend service skeleton [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #3912:
URL: https://github.com/apache/texera/pull/3912#discussion_r2443794478
##########
chat-assistant-service/pyproject.toml:
##########
@@ -0,0 +1,23 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[tool.black]
+line-length = 88
+target-version = ["py312"]
+
+[tool.flake8]
+max-line-length = 88
Review Comment:
I read the doc you linked. If toml is the way to go let's use it! Thanks for letting me know!
I wonder if you can update the linter/formater configuration for pyamber as well, to align with this way?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(chat-assistant-service): add backend service skeleton [texera]
Posted by "Xiao-zhen-Liu (via GitHub)" <gi...@apache.org>.
Xiao-zhen-Liu commented on code in PR #3912:
URL: https://github.com/apache/texera/pull/3912#discussion_r2445668042
##########
chat-assistant-service/requirements.txt:
##########
Review Comment:
To clarify, for python projects (this one and PyAmber), we use requirements.txt for dependency management. We only use `pyproject.toml` to set linter and formatter configurations, although it can be used for many other things, including managing packages.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(chat-assistant-service): add backend service skeleton [texera]
Posted by "Xiao-zhen-Liu (via GitHub)" <gi...@apache.org>.
Xiao-zhen-Liu commented on code in PR #3912:
URL: https://github.com/apache/texera/pull/3912#discussion_r2441456543
##########
chat-assistant-service/pyproject.toml:
##########
@@ -0,0 +1,23 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[tool.black]
+line-length = 88
+target-version = ["py312"]
+
+[tool.flake8]
+max-line-length = 88
Review Comment:
[black configuration](https://black.readthedocs.io/en/stable/usage_and_configuration/the_basics.html#command-line-options) can only be set via `pyproject.toml`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(chat-assistant-service): add backend service skeleton [texera]
Posted by "Xiao-zhen-Liu (via GitHub)" <gi...@apache.org>.
Xiao-zhen-Liu commented on PR #3912:
URL: https://github.com/apache/texera/pull/3912#issuecomment-3470440020
We have decided to merge another prototype instead of this one, so I will close this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: [chat-assistant-service][1] add backend service skeleton [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #3912:
URL: https://github.com/apache/texera/pull/3912#issuecomment-3413549675
please avoid using `[[chat-assistant-service][1]` this syntax.
1. we have agreed to follow conversional commits, you can use `feat(chat-assistant-service)`
2. it is generally not a good idea to label 1, 2, 3 in PRs, as this is a shared repo, your [2] != other's [2], could be confusing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(chat-assistant-service): add backend service skeleton [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #3912:
URL: https://github.com/apache/texera/pull/3912#discussion_r2438295705
##########
chat-assistant-service/pyproject.toml:
##########
@@ -0,0 +1,23 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[tool.black]
+line-length = 88
+target-version = ["py312"]
+
+[tool.flake8]
+max-line-length = 88
Review Comment:
let's use explicit config file (e.g., `.flake` for flake8 configuration), instead of .toml?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org