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

🧪 Create PubChem Database Importer Script #2

Closed
2 tasks
nesimtunc opened this issue Jan 30, 2022 · 14 comments · Fixed by #16
Closed
2 tasks

🧪 Create PubChem Database Importer Script #2

nesimtunc opened this issue Jan 30, 2022 · 14 comments · Fixed by #16
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@nesimtunc
Copy link
Member

Please Read The WikiPages to complete script's requirements.

@nesimtunc nesimtunc created this issue from a note in Periodum Backend & API (To do) Jan 30, 2022
@cagrimertbakirci cagrimertbakirci added this to To do in Periodum Backend & API via automation Jan 30, 2022
@nesimtunc nesimtunc added good first issue Good for newcomers help wanted Extra attention is needed labels Jan 30, 2022
@nesimtunc nesimtunc pinned this issue Jan 30, 2022
@nesimtunc nesimtunc changed the title Create PubChem Database Importer Script 🧪 Create PubChem Database Importer Script Feb 3, 2022
@nesimtunc nesimtunc added this to the Initial Release milestone Feb 5, 2022
@T410
Copy link
Contributor

T410 commented Feb 7, 2022

Hey everyone,
I have been developing a node project for this issue for a "weekend project"
https://github.com/T410/pubchem-fetch

There are some small changes to implement, and of course, the rate limiter should be implemented as well. I will get into those after my working hours.

Here's the JSON stripped from unwanted information.
https://github.com/T410/pubchem-fetch/blob/main/compounds/222_reduced.json

After the development is done I can hand the ownership of the repo to you.

I know the code looks a bit messy. I will refactor it

You can test it by running
node dist/index.js startId endId compoundId
node dist/index.js 0 5 222

@nesimtunc
Copy link
Member Author

Hey @T410 !

Thank you very much! That's a lot of hard working! We appreciate your effort! Looking forward to implement this on our code base step by step.

@nesimtunc
Copy link
Member Author

Btw, since this task is quite complicated, it would be very helpful if you could implement some tests after this PR merged please.

Thank you! 🙏🏼

@T410
Copy link
Contributor

T410 commented Feb 7, 2022

I initially thought, this script will be running externally, independent from the backend project. So that's why I have created a repo for this. Just to let you know.

I will be more than happy to help you integrate the repo into the codebase.

Also yes, I need to write tests for the project. But for that I may need a little assistance, maybe. Not sure.

This is the least I can do for Evrim Ağacı. I appreciate what you guys do and I want you to know your efforts are much appreciated as well

@nesimtunc nesimtunc moved this from To do to In progress in Periodum Backend & API Feb 7, 2022
@T410
Copy link
Contributor

T410 commented Feb 9, 2022

Hey @nesimtunc,

I have done some refactoring and manually tested the code. It looks ok. However, I still need to write the rate limiter.

What are your thoughts on implementing the script into the backend? How should we approach it? Should we just squeeze the script files in and create a field in the scripts section of package.json? Or do you have something else on your mind?

EDIT: I see the PR you mentioned has been merged. I will write tests after writing the rate limiter logic as well. No worries about that 👍

Thanks and kolay gelsin 🙌

@nesimtunc
Copy link
Member Author

Hi @T410,

Thank you very much!

Sorry, for late the reply and this won't be a proper comment as well. I'll be reviewing the codes by Friday. I might have some feedbacks. But yes, it will be part of the app script (like you mentioned, in package.json) could be something like this. import:pubchem so we will execute it from command line npm run import:pubchem {startId} {endId} manually.

@T410
Copy link
Contributor

T410 commented Feb 10, 2022

Hey @nesimtunc,

Some update:

I have implemented the rate limiter logic.
I forked the repo. Will implement the code into the repo. That will be probably done on the weekend. After that, I will write the tests and create a PR for you to review.

This is/will be my first contribution to a public repo, so thank you for bearing with me 🙏

@nesimtunc
Copy link
Member Author

nesimtunc commented Feb 11, 2022

Hey @T410

Sorry again, I couldn't get a chance to take a look to your code. :(

Also, there's some update from me today, I started to integrate Prisma using MongoDB but due to Preview, there was so many things we wouldn't be able to do, like limited field type and sub document which is important part of MongoDB, so, I had to back to MySQL using Prisma, and here's a simple sample, you may merge my branch into yours and using Prisma to save the records to the db. But I guess you might need to update the Prisma's schema, feel free to do so.

#13

@nesimtunc
Copy link
Member Author

Hi @T410,

#13 has been done.

@T410
Copy link
Contributor

T410 commented Feb 17, 2022

Hi @nesimtunc,

Sorry, I haven't been able to find time to continue the development. I will continue working on the issue today evening

@nesimtunc
Copy link
Member Author

No worries @T410, just wanted to let you know. Please take your time as this is a volunteer work we all do. Thank you for your hardworking. Appreciated it.

@cagrimertbakirci
Copy link
Member

@T410 @nesimtunc This progress is really wonderful, thank you so much for your contributions. It is really special to see how hard you guys work in such limited time. Thank you! ♥

@T410 T410 mentioned this issue Feb 28, 2022
@asiminnesli
Copy link

this "issue" is still a issue? or solved already?

if issue is not solved. i will work on this.

thanks

@T410
Copy link
Contributor

T410 commented Mar 17, 2022

Hey @asiminnesli

I am working on this. Most of the script is basically done. I am writing the tests at the moment. I couldn't create a time to work on this lately but I will focus more on this issue this weekend.

@nesimtunc

@nesimtunc nesimtunc linked a pull request Mar 23, 2022 that will close this issue
Periodum Backend & API automation moved this from In progress to Done Apr 27, 2022
@nesimtunc nesimtunc unpinned this issue May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
Development

Successfully merging a pull request may close this issue.

4 participants