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.

Sign up for GitHub

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

Jump to bottom

Enable https #64

Merged
merged 13 commits into from
Oct 16, 2021
Merged

Enable https #64

merged 13 commits into from
Oct 16, 2021

Conversation

AnthonyLaw
Copy link
Member

Added

  • support HTTP / HTTPS request
  • add finalization and node status

fix

  • request node status timeout.

@AnthonyLaw AnthonyLaw requested review from OlegMakarenko and yilmazbahadir and removed request for OlegMakarenko and yilmazbahadir October 9, 2021 09:27
@OlegMakarenko OlegMakarenko requested review from OlegMakarenko and removed request for yilmazbahadir October 13, 2021 13:48
fboucquez
fboucquez reviewed Oct 13, 2021
View reviewed changes
src/services/ApiNodeService.ts Outdated
return {
isAvailable: false,
lastStatusCheck: Date.now(),
};
}
};

static getNodeInfo = async (host: string, port: number): Promise<NodeInfo | null> => {
static getNodeInfo = async (host: string, port: number, protocol = 'http'): Promise<NodeInfo | null> => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't use the default "http" value, make it required. You want to be sure you are always sending what you want. For example, you can miss that the port is 3001 but using the the default http value.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, we may miss that node still in `HTTP right?

fboucquez
fboucquez reviewed Oct 13, 2021
View reviewed changes
.travis.yml Show resolved Hide resolved
@yilmazbahadir
Copy link
Contributor

Good work @AnthonyLaw ! Looks good to be merged (after Fer's comments are resolved).

p.s. Initially was concerned about the potential overlapping timers issue but when I checked the implementation saw that it's waiting for interval seconds before the next round, therefore, preventing a potential overlap issue.

@AnthonyLaw AnthonyLaw merged commit 9e845ed into dev Oct 16, 2021
@AnthonyLaw AnthonyLaw deleted the enable-https branch October 16, 2021 11:05
AnthonyLaw added a commit that referenced this pull request Oct 30, 2021
@AnthonyLaw @OlegMakarenko
Dev -> Main (#78)
f63adaf 
* Enable https (#64)

* refactor: add clearTimeout in get request

* style: remove one line code

* feat: add https checker

* fix: split nodos before init promise.

* fix: add protocol support when request node info status

* feat: add node health

* feat: add finalization

* style: fix lint, and disable unuse build script

* fix: change params to required

Co-authored-by: OlegMakarenko <33131259+OlegMakarenko@users.noreply.github.com>

* refactor: removed node reward related code (#68)

Co-authored-by: OlegMakarenko <33131259+OlegMakarenko@users.noreply.github.com>

* Nodes filter (#70)

* feat: add PREFERRED_NODES in config

* fix: data type

* feat: add filter on nodes endpoint

* fix: filter preferred return api node (#72)

* Add limit and client version filter (#73)

* feat: add limit on node list search

* feat: added client version filter

* style: fix lint

* fix: change request

* docs: release note (#77)

Co-authored-by: OlegMakarenko <33131259+OlegMakarenko@users.noreply.github.com>
AnthonyLaw added a commit that referenced this pull request Oct 30, 2021
@AnthonyLaw @OlegMakarenko
Improve filter (#76)
093ae3e 
* Dev -> Main (#78)

* Enable https (#64)

* refactor: add clearTimeout in get request

* style: remove one line code

* feat: add https checker

* fix: split nodos before init promise.

* fix: add protocol support when request node info status

* feat: add node health

* feat: add finalization

* style: fix lint, and disable unuse build script

* fix: change params to required

Co-authored-by: OlegMakarenko <33131259+OlegMakarenko@users.noreply.github.com>

* refactor: removed node reward related code (#68)

Co-authored-by: OlegMakarenko <33131259+OlegMakarenko@users.noreply.github.com>

* Nodes filter (#70)

* feat: add PREFERRED_NODES in config

* fix: data type

* feat: add filter on nodes endpoint

* fix: filter preferred return api node (#72)

* Add limit and client version filter (#73)

* feat: add limit on node list search

* feat: added client version filter

* style: fix lint

* fix: change request

* docs: release note (#77)

Co-authored-by: OlegMakarenko <33131259+OlegMakarenko@users.noreply.github.com>

* Creating new version 1.1.1

* feat: added restGatewayUrl field

* feat: add ssl node query

* fix: extra ":" string cause the query issue

Co-authored-by: OlegMakarenko <33131259+OlegMakarenko@users.noreply.github.com>
Co-authored-by: Travis CI User <travis@example.org>
fboucquez pushed a commit that referenced this pull request Oct 31, 2021
@AnthonyLaw @OlegMakarenko @fboucquez
Enable https (#64)
6f7a8e4 
* refactor: add clearTimeout in get request

* style: remove one line code

* feat: add https checker

* fix: split nodos before init promise.

* fix: add protocol support when request node info status

* feat: add node health

* feat: add finalization

* style: fix lint, and disable unuse build script

* fix: change params to required

Co-authored-by: OlegMakarenko <33131259+OlegMakarenko@users.noreply.github.com>
fboucquez pushed a commit that referenced this pull request Oct 31, 2021
@AnthonyLaw @OlegMakarenko @fboucquez
Improve filter (#76)
8a505e9 
* Dev -> Main (#78)

* Enable https (#64)

* refactor: add clearTimeout in get request

* style: remove one line code

* feat: add https checker

* fix: split nodos before init promise.

* fix: add protocol support when request node info status

* feat: add node health

* feat: add finalization

* style: fix lint, and disable unuse build script

* fix: change params to required

Co-authored-by: OlegMakarenko <33131259+OlegMakarenko@users.noreply.github.com>

* refactor: removed node reward related code (#68)

Co-authored-by: OlegMakarenko <33131259+OlegMakarenko@users.noreply.github.com>

* Nodes filter (#70)

* feat: add PREFERRED_NODES in config

* fix: data type

* feat: add filter on nodes endpoint

* fix: filter preferred return api node (#72)

* Add limit and client version filter (#73)

* feat: add limit on node list search

* feat: added client version filter

* style: fix lint

* fix: change request

* docs: release note (#77)

Co-authored-by: OlegMakarenko <33131259+OlegMakarenko@users.noreply.github.com>

* Creating new version 1.1.1

* feat: added restGatewayUrl field

* feat: add ssl node query

* fix: extra ":" string cause the query issue

Co-authored-by: OlegMakarenko <33131259+OlegMakarenko@users.noreply.github.com>
Co-authored-by: Travis CI User <travis@example.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yilmazbahadir yilmazbahadir yilmazbahadir left review comments

@fboucquez fboucquez fboucquez approved these changes

@OlegMakarenko OlegMakarenko Awaiting requested review from OlegMakarenko

Assignees

@yilmazbahadir yilmazbahadir

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AnthonyLaw @yilmazbahadir @fboucquez @OlegMakarenko